Skip to content

Conversation

prdoyle
Copy link
Contributor

@prdoyle prdoyle commented Apr 21, 2025

Having separated the responsibilities of PolicyManager and ScopeResolver in #126921, this PR now separates the testing responsibilities:

  • ScopeResolverTests ensures that we determine the right PolicyScope information for a given class
  • PolicyManagerTests ensures that we determine the right policy for a given PolicyScope

This has a dramatic simplifying effect on the tests.

I've also removed some redundant tests: we had some tests for one-module policies and others for two-module policies; but the two-module tests provide complete coverage on their own.

@prdoyle prdoyle added >test Issues or PRs that are addressing/adding tests auto-backport Automatically create backport pull requests when merged v8.18.1 v8.19.0 v9.0.1 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure labels Apr 21, 2025
@prdoyle prdoyle self-assigned this Apr 21, 2025
@prdoyle prdoyle requested a review from a team as a code owner April 21, 2025 15:19
@prdoyle prdoyle marked this pull request as draft April 21, 2025 15:19
@prdoyle
Copy link
Contributor Author

prdoyle commented Apr 21, 2025

Draft PR because it builds on #126921.

@prdoyle prdoyle force-pushed the scope-resolver-tests branch from b19289e to 915958e Compare April 21, 2025 16:11
@prdoyle prdoyle marked this pull request as ready for review April 21, 2025 16:58
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Apr 21, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@prdoyle prdoyle merged commit 4d929ca into elastic:main Apr 23, 2025
17 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.18
8.x Commit could not be cherrypicked due to conflicts
9.0

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 127115

prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request Apr 23, 2025
* Simplify PolicyManagerTests

* Clean and simplify ScopeResolverTests
prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request Apr 23, 2025
* Simplify PolicyManagerTests

* Clean and simplify ScopeResolverTests
@prdoyle
Copy link
Contributor Author

prdoyle commented Apr 23, 2025

8.x was blocked by this guy.

@prdoyle prdoyle deleted the scope-resolver-tests branch April 23, 2025 13:09
elasticsearchmachine pushed a commit that referenced this pull request Apr 23, 2025
* Simplify PolicyManagerTests

* Clean and simplify ScopeResolverTests
elasticsearchmachine pushed a commit that referenced this pull request Apr 23, 2025
* Simplify PolicyManagerTests

* Clean and simplify ScopeResolverTests
prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request May 29, 2025
…tic#127238)

* Simplify PolicyManagerTests

* Clean and simplify ScopeResolverTests
prdoyle added a commit that referenced this pull request May 30, 2025
…128645)

* Simplify PolicyManagerTests

* Clean and simplify ScopeResolverTests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure Team:Core/Infra Meta label for core/infra team >test Issues or PRs that are addressing/adding tests v8.18.1 v8.19.0 v9.0.1 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants