Enable the remaining slow-group tests in the default build#702
Open
vharseko wants to merge 3 commits into
Open
Enable the remaining slow-group tests in the default build#702vharseko wants to merge 3 commits into
vharseko wants to merge 3 commits into
Conversation
Move all remaining tests out of the "slow" TestNG group so that they run in the default build: the group has been excluded from CI since the Maven migration, and the full audit of the group showed these tests are green. Also enable the two remaining disabled tests, both verified working: - PostConnectedDisconnectTest.checkPostConnectDisconnectPlugin (~15s); - AssuredReplicationServerTest.testSafeReadOneRSComplex, disabled historically as "working but 17.5 minutes to run": it completes its 240 combinations in about 6 minutes on current hardware. Raise forkedProcessTimeoutInSeconds from 900 to 2400: the fully enabled AssuredReplicationServerTest takes about 20 minutes and no longer fits the old watchdog. The excludegroups=slow filter itself is kept for now: the slow annotations remaining in files owned by the other pull requests of this series must not be enabled before those fixes are merged; a final cleanup can drop the filter once the series has landed. Files touched by the other pull requests of the series are left untouched here to avoid merge conflicts.
Once the whole series of slow-group pull requests is merged, no test carries the "slow" TestNG group anymore, so the excludegroups filter is dead configuration. This makes this pull request the last one of the series to merge: applying it before the others would prematurely enable the not-yet-fixed slow tests still present in their files.
… tests A freshly connected peer starts in catch-up mode: until its server writer has checked there is nothing to recover from the changelog DB, updates are re-read from the DB and keep the assured flag of the original sender - the NotAssuredUpdateMsg substitution only applies to the in-memory queue path. An update sent by the test in that window reaches fake DSs/RSs with unexpected assured parameters and fails checkUpdateAssuredParameters(). This is why testSafeDataLevelOne and testSafeReadOneRSComplex failed stochastically on every CI job while passing locally. waitForStableTopo() now also waits until every peer connected to the real RS is "following" (new package-private MessageHandler.isFollowing() accessor), and testSafeDataLevelOne now calls it before sending updates. 357/357 green locally in ~21 min.
vharseko
added
tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The
slowTestNG group has been excluded from CI since the Maven migration (excludegroups=slowin the failsafe configuration), so none of its 42 classes / 78 test methods ever ran. A full audit of the group (see #684-#700 for the fixes it produced) showed the remaining tests are green.This pull request must be merged LAST in the series (#675, #684, #685, #686, #687, #688, #689, #691, #694, #698, #700): it drops the
excludegroups=slowfilter entirely, so merging it earlier would prematurely enable the not-yet-fixed slow tests still present in the files owned by the pending pull requests.Changes
groups="slow"annotation is removed so the tests run in the default build (entry cache tests keep their secondarytest*CacheConcurrencygroup labels). Files owned by the other pull requests of this series are deliberately left untouched to avoid merge conflicts — their formerly-slow tests arrive with those branches.PostConnectedDisconnectTest.checkPostConnectDisconnectPlugin(~15s, green);AssuredReplicationServerTest.testSafeReadOneRSComplex, disabled historically as "working but disabled as 17.5 minutes to run": its 240 combinations complete in ~6 minutes on current hardware (240/240 green).forkedProcessTimeoutInSecondsraised 900 → 2400: the fully enabledAssuredReplicationServerTesttakes ~20 minutes and no longer fits the old per-fork watchdog.excludegroups=slowproperty is removed from the failsafe configuration — after the series lands, no test carries theslowgroup anymore, so the filter is dead configuration (hence the merge-order requirement above).Verification (default CI group filter)
PostConnectedDisconnectTest1/1;AssuredReplicationServerTestfull class 357/357 in ~20 minutes (including the 240-case complex scenario);GenerationIdTest4/4,InitOnLineTest9/9,PrivilegeTestCase182/182,TestCrypt4257/4257;CI cost: roughly +30 minutes per matrix job, dominated by
AssuredReplicationServerTest(~20 min).