-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Allow REST tests to run in MP mode #126906
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
Merged
Merged
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
b38bc7f
Allow REST tests to run in MP mode
nielsbauman 2b2781f
Merge branch 'main' into mp-rest-tests
nielsbauman 3054917
Fix MP tests
nielsbauman b1ed397
Re-add some lines
nielsbauman a4400f7
Move some methods
nielsbauman 4f66b0a
Parse boolean correctly
nielsbauman 9ea60ea
Refactor client settings
nielsbauman a7c4add
More fixes
nielsbauman 3bac353
Another fix
nielsbauman b44b218
Fixups
nielsbauman fbb6989
Merge branch 'main' into mp-rest-tests
nielsbauman ada76f3
Merge remote-tracking branch 'upstream/main' into mp-rest-tests
nielsbauman f8d722d
Merge remote-tracking branch 'upstream/main' into mp-rest-tests
nielsbauman c30fab6
Merge branch 'main' into mp-rest-tests
nielsbauman bf26c1b
Add `MultiProjectEnabledClusterConfigProvider`
nielsbauman 4a140c1
Merge branch 'main' into mp-rest-tests
nielsbauman 9616890
Fix some more tests
nielsbauman 0fe1aa7
Uncomment line
nielsbauman f637792
Merge branch 'main' into mp-rest-tests
nielsbauman e35fe72
Readd dependency to fix tests
nielsbauman cb3ae3b
Readd MP module and setting to MP-only tests
nielsbauman d431975
Merge branch 'main' into mp-rest-tests
nielsbauman 2af86c8
Don't include MP module
nielsbauman 8c55b7f
Run MP ITs on INTEG_TEST distrib
nielsbauman 0d8add0
Merge branch 'main' into mp-rest-tests
nielsbauman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
26 changes: 26 additions & 0 deletions
26
...n/java/org/elasticsearch/test/cluster/local/MultiProjectEnabledClusterConfigProvider.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.test.cluster.local; | ||
|
|
||
| public class MultiProjectEnabledClusterConfigProvider implements LocalClusterConfigProvider { | ||
|
|
||
| @Override | ||
| public void apply(LocalClusterSpecBuilder<?> builder) { | ||
| if (isMultiProjectEnabled()) { | ||
| builder.setting("test.multi_project.enabled", "true").module("test-multi-project"); | ||
| } | ||
| } | ||
|
|
||
| private static boolean isMultiProjectEnabled() { | ||
| // TODO: we need to use `tests` instead of `test` here to make gradle passes the system property, | ||
| // but we need `test` in the setting. | ||
| return Boolean.getBoolean("tests.multi_project.enabled"); | ||
| } | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was that changed? Using DEFAULT is build wise quite more expensive to run as it requires all plugins and modules to be build as part of this. In general we should aim to use the INTEG_TEST distribution where possible. If there's any convenience we can add to make that easier, lets do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I mainly removed it because of the TODO and the tests passed. I'll revert the changes and remove the comments.
The
DistributionTypenames are unintuitive to me. I asked about this in #es-elivery a few days ago so now I understand what the difference is why we actually wantINTEG_TEST. I think a first step to make people move away fromDEFAULTtoINTEG_TESTis to rename the values.DEFAULTcould maybe beALL_MODULESwith a JavaDoc explaining why you shouldn't use that. ThenINTEG_TESTcould maybe beDEFAULTwith a JavaDoc explaining that you need to add modules yourself (plus an explanation why we want people to use the latter). These are just suggestions, my point is that not wanting people to use something calledDEFAULTwithout any docs (except for an email that people like me might have missed) is a bit confusing.