Skip to content

Issue #2989 [DefaultPathProcessor] Implementation-dependent tests in testVanityUrl and testVanityConfig#2991

Open
kabo87777 wants to merge 3 commits intoadobe:mainfrom
kabo87777:fix-124
Open

Issue #2989 [DefaultPathProcessor] Implementation-dependent tests in testVanityUrl and testVanityConfig#2991
kabo87777 wants to merge 3 commits intoadobe:mainfrom
kabo87777:fix-124

Conversation

@kabo87777
Copy link

@kabo87777 kabo87777 commented Oct 1, 2025

Q A
Fixed Issues? Fixes #2989
Patch: Bug Fix? Yes
Minor: New Feature?
Major: Breaking Change?
Tests Added + Pass? Yes
Documentation Provided Yes (code comments and or markdown)
Any Dependency Changes?
License Apache License, Version 2.0

Fix ID(Implementation-dependent) tests in DefaultPathProcessorTest

Solution

For testVanityUrl:

  • Added explicit check for Externalizer service availability before component registration
  • Implemented fallback mock registration to ensure dependency is always satisfied
  • Maintains original test logic while eliminating timing dependencies

For testVanityConfig:

  • Replaced non-deterministic assertions with controlled mock ResourceResolver instances
  • Eliminated dependency on CoreComponentTestContext mapping order
  • Used explicit mocking to ensure predictable test outcomes

Verification

  • NonDex testing: All seeds now pass consistently (previously failed on 2/3 seeds)
  • Functional equivalence: Tests verify the same behavior with deterministic setup
  • No regressions: All existing test functionality preserved

This fix ensures the test suite remains robust and reliable, supporting the overall quality of the AEM Core Components project.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement here, I like the direction of making these tests less dependent on implementation details.

One small thought: vanity URL tests can get flaky if they rely on implicit ordering or default configs from the test environment. Maybe we can make the setup a bit more explicit so the expected result is fully deterministic (especially if multiple vanity values exist).

Not a blocker at all, just something that might help avoid future CI surprises. Overall this looks good to me.

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.

[DefaultPathProcessor] Implementation-dependent tests in testVanityUrl and testVanityConfig

2 participants