Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-21776: Admin section: make the Notifications section pass webstandard tests #2801

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Jan 17, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-21776

Changes

Description

  • Added a fallback for null value of $divData
  • Added a label for the id="since" input at the end of the form
  • Added a missing space between attributes in email preferences
  • Replaced the comment with an actual test for Notifications

Screenshots & Video

No visual change.

Executed Tests

  • Successfully passed mvn clean install -f xwiki-platform-distribution/xwiki-platform-distribution-flavor/xwiki-platform-distribution-flavor-test/xwiki-platform-distribution-flavor-test-webstandards -Dxwiki.test.startXWiki=false after updating the live distribution with the changes in this PR.

Expected merging strategy

  • Prefers squash: Yes
  • Merge only on master.

…andard tests

* Added a fallback for null value of `$divData`
* Added a label for the `id="since"` input at the end of the form
* Added a missing space between attributes in email preferences
* Replaced the comment with an actual test for Notifications
@surli
Copy link
Member

surli commented Jan 18, 2024

Could you run xwiki-platform-notification-test-docker with your changes here?

@Sereza7
Copy link
Contributor Author

Sereza7 commented Feb 13, 2024

After updating my builds of the changes with mvn clean install -f xwiki-platform-core/xwiki-platform-legacy/xwiki-platform-legacy-events-hibernate/xwiki-platform-legacy-events-hibernate-ui and mvn clean install -f xwiki-platform-core/xwiki-platform-notifications/xwiki-platform-notifications-ui, I launched the build mvn clean install -amd -f xwiki-platform-core/xwiki-platform-notifications/xwiki-platform-notifications-test -pl :xwiki-platform-notifications-test-docker -Pquality,integration-tests,docker. It had one failure, on AllIT$NestedNotificationsEmailsIT. IMO this failure is not related to the changes in this PR. Here is a paste of the failsafe logs.

@Sereza7 Sereza7 marked this pull request as draft February 13, 2024 12:32
@Sereza7
Copy link
Contributor Author

Sereza7 commented Feb 13, 2024

I can't reproduce this fail on master. Need to fix it before merging.

…andard tests

* Replaced other uses of Divdata to account for the fact that it might be null
@Sereza7
Copy link
Contributor Author

Sereza7 commented Feb 14, 2024

Okay, it seems it was a fluke. After further analysis, it didn't seem like my changes were related to the error.
I reran the whole command: mvn clean install -f xwiki-platform-core/xwiki-platform-legacy/xwiki-platform-legacy-events-hibernate/xwiki-platform-legacy-events-hibernate-ui; mvn clean install -f xwiki-platform-core/xwiki-platform-notifications/xwiki-platform-notifications-ui; mvn clean install -amd -f xwiki-platform-core/xwiki-platform-notifications/xwiki-platform-notifications-test -pl :xwiki-platform-notifications-test-docker -Pquality,integration-tests,docker; spd-say "DONE" and this time around it just failed on a timeout like the master branch...
I probably forgot to update my local branch.

[ERROR] Errors: 
[ERROR]   AllIT$NestedNotificationsIT>NotificationsIT.ownEventNotifications:408 » Timeout Timeout while waiting [20] sec on notification count. Expected: [1] - Latest value received: [0].
[INFO] 
[ERROR] Tests run: 12, Failures: 0, Errors: 1, Skipped: 0

@Sereza7 Sereza7 marked this pull request as ready for review February 14, 2024 15:48
@surli surli merged commit a0174ab into xwiki:master Feb 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants