Fix two broken AciTests cases and enable the suite in the default build#675
Merged
vharseko merged 2 commits intoJul 8, 2026
Merged
Conversation
AciTests (515 tests covering the dseecompat ACI machinery) is in the "slow" TestNG group, which the build excludes, so the suite never ran in CI and two tests silently rotted: - testValidAcis declared an ACI with targattrfilters attribute "1sn_" valid. The name passes the deliberately lenient dseecompat ATTR_NAME pattern, but the filter part is now parsed as an RFC 4512 attribute description, where a leading digit means a numeric OID, so the ACI is rejected at decode time (result 21). No schema can define such an attribute, so strict rejection is correct: the case moves to the invalidAcis provider. - testCompare expected the ldapcompare tool to exit with COMPARE_TRUE (6), but the tool returns 0 for a successful true comparison unless --useCompareResultCode is given. Pass the flag so the test asserts the actual comparison result again. Remove the "slow" group from the AciTests class so the suite runs in the default build (~34 s); other slow-group classes are unaffected.
maximthomas
approved these changes
Jul 3, 2026
This was referenced Jul 5, 2026
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
2 participants
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.
Summary
AciTests— the 515-test suite covering the whole dseecompat ACI machinery(parsing, targets, bind rules, effective rights, proxy auth) — is in the
slowTestNG group, which the build excludes (excludegroups=slowin thefailsafe configuration). The suite therefore never runs in CI, and two
tests silently rotted. Found while validating #674 against the full suite.
The two failures
testValidAcisdeclared this ACI valid:The attribute name
1sn_passes the deliberately lenient dseecompatATTR_NAMEpattern (Aci.javadocuments it as "graciously" matchingnames beginning with a letter or digit), but the filter part
(1sn_=joe*)is parsed via the SDK's RFC 4512AttributeDescriptionparser, where a token starting with a digit must be a numeric OID — so
decoding fails ("invalid character 's' at position 1") and the ADD is
rejected with result 21. Since no schema can define an attribute named
1sn_", an ACI referencing it can never match anything; strict rejection is the correct behaviour, and the test case moves to theinvalidAcis`provider (with a comment explaining why).
testCompareexpected theldapcomparetool to exit withCOMPARE_TRUE(6), but the toolkit tool returns 0 for a successfultrue comparison unless
--useCompareResultCodeis given. The test nowpasses the flag, so it again asserts the actual comparison result
(TRUE → 6, FALSE → 5) rather than a generic success.
Enabling the suite
The
groups="slow"marker is removed from theAciTestsclass-level@Testannotation, so the suite runs in the default build. It adds ~34 s.The pom-level
excludegroups=slowis left untouched — the other ~40slow-group classes (replication stress tests, quicksetup, etc.) are not
affected.
Testing
mvn -P precommit -pl opendj-server-legacy verify -Dit.test=AciTestswiththe default configuration (no group overrides): 515 tests, 0 failures,
0 skipped, ~34 s.
Files
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/AciTests.java