Allow BWC Testing against a specific branch #25510
Conversation
rjernst
left a comment
rjernst
left a comment
There was a problem hiding this comment.
I left some comments. I think the sys prop should also be called tests.bwc.refspec since it is including the remote (eg look at git help for git fetch, it takes refspec, not branch).
distribution/bwc/build.gradle
Outdated
| if (checkoutDir.exists() == false) { | ||
| commandLine = ['git', 'clone', rootDir, checkoutDir] | ||
| } else { | ||
| commandLine = ['git', 'fetch', 'origin'] |
There was a problem hiding this comment.
I don't think this is correct. A clone should always be created. The origin remote only comes into play here because you know origin is the local checkout the bwc checkout was cloned from.
There was a problem hiding this comment.
I'm not sure I follow - the previous logic checked if the directory existed and skipped cloning?
distribution/bwc/build.gradle
Outdated
| dependsOn fetchLatest | ||
| def String branch = System.getProperty("tests.bwc.branch", "upstream/${bwcBranch}") | ||
| if (branch.startsWith('upstream')) { | ||
| dependsOn fetchLatest |
There was a problem hiding this comment.
I think the fetchLatest should remain. I think we want to simply update fetchLatest to do a git fetch instead of git fetch upstream? Then whatever the specified ref is here (whether the default based on upstream or a local one based on origin) should be available.
There was a problem hiding this comment.
That's a good idea - it simplifies things and make them more powerful. I'll do that. Thanks.
|
@rjernst I applied your idea. Can you take another look? |
This is a regression introduced in #25510, which removed the explicit fetching of upstream. Sadly this doesn't work if you don't have any local branch referring to `upstream` as an upstream branch.
* master: (42 commits) Harden global checkpoint tracker Remove deprecated created and found from index, delete and bulk (elastic#25516) fix testEnsureVersionCompatibility for 5.5.0 release fix Version.v6_0_0 min compatibility version to 5.5.0 Add bwc indices for 5.5.0 Add v5_5_1 constant [DOCS] revise high level client Search Scroll API docs (elastic#25599) Improve REST error handling when endpoint does not support HTTP verb, add OPTIONS support (elastic#24437) Avoid SecurityException in repository-S3 on DefaultS3OutputStream.flush() (elastic#25254) [Tests] Add tests for CompletionSuggestionBuilder#build() (elastic#25575) Enable cross-setting validation [Docs] Fix typo in bootstrap-checks.asciidoc (elastic#25597) Index ids in binary form. (elastic#25352) bwc checkout should fetch from all remotes IndexingIT should check for global checkpoints regardless of master version [Tests] Add tests for PhraseSuggestionBuilder#build() (elastic#25571) Remove unused class MinimalMap (elastic#25590) [Docs] Document Scroll API for Java High Level REST Client (elastic#25554) Disable date field mapping changing (elastic#25285) Allow BWC Testing against a specific branch (elastic#25510) ...
4 participants
Some times we need a fix / change to have two parts in two different branches (corresponding to two different ES releases). In order to be able to test these cases you need to run the BWC tests against a local branch rather than then using a branch from
github.com/elastic/elasticsearch.This PR adds a system property called
tests.bwc.branchthat allows you to do it. Note that I've chosen to go with the simplest code change for now, at the expense of some user friendliness. As follow ups, we can do some more git magic to make things nicer if people prefer