-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[test] add java packaging test project #30161
[test] add java packaging test project #30161
Conversation
Adds a project for building and running packaging tests written in java for portability. The vagrant tasks use jars on the packagingTest configuration, which are built in the same project. No tests are added yet. Corresponding changes are not made to :x-pack:qa:vagrant because the java packaging tests will all be consolidated into one project. For elastic#26741
Pinging @elastic/es-core-infra |
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.
Looks good, I just have a couple questions.
description 'Clean the project build directory' | ||
group 'Build' | ||
delete project.buildDir | ||
if (project.tasks.findByName('clean') == null) { |
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.
In what case will this already be created? Can we rely on it already being created and remove this altogether?
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.
The java plugin also adds a clean task, and it complains when we try to add another one. My thinking was that I didn't want to bake in the expectation that the project vagrant test plugin is applied to is also a java project. For example :x-pack:qa:vagrant
has the vagrant test plugin but not the java plugin
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.
Hmmm. Maybe maybe it'd be simpler to create a noop java test in there in this PR instead of doing this? I imagine we'll want to make java tests in there eventually anyway.
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.
I believe we're going to put all the java packaging tests (including x-pack) into :qa:vagrant
, and once all the bats tests are ported we can remove :x-pack:qa:vagrant
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.
Ah. That makes sense to me.
Task createTestRunnerScript = project.tasks.create('createTestRunnerScript', FileContentsTask) { | ||
dependsOn copyPackagingTests | ||
file "${testsDir}/run-tests.sh" | ||
contents "java -cp \"*\" org.junit.runner.JUnitCore ${-> project.extensions.esvagrant.testClass}" |
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.
Should this have a shebang line?
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.
Alternatively, if we just always run as bash /path/to/run-tests.sh
then we don't need the executable stuff?
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.
Yeah running with bash
is more straightforward, I'll do that since we already have the expectation that bash is present on all boxes. It also maps more cleanly to how it'll work with powershell
Changes from review comments
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.
LGTM
qa/vagrant/build.gradle
Outdated
compile "org.hamcrest:hamcrest-core:${versions.hamcrest}" | ||
|
||
// needs to be on the classpath for JarHell | ||
runtime project(':libs:elasticsearch-core') |
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.
Shouldn't this just be testRuntime?
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.
Yeah that makes sense, but it's going to change to compile
once we start adding tests because we're going to use some of the filesystem stuff in IOUtils
Jenkins test this please |
Jenkins run gradle build tests please |
* origin/master: [test] add java packaging test project (elastic#30161) Fix macros in changelog (elastic#30269) [DOCS] Fixes syskeygen command name [ML] Include 3rd party C++ component notices (elastic#30132) _cluster/state Skip Test for pre-6.4, not pre-7.0 (elastic#30264) Improve docs for disk watermarks (elastic#30249) [DOCS] Removes redundant Active Directory realm settings (elastic#30190) [DOCS] Removes redundant LDAP realm settings (elastic#30193) _cluster/state should always return cluster_uuid (elastic#30143) HTML5ify Javadoc for core and test framework (elastic#30234) Minor tweaks to reroute documentation (elastic#30246)
[test] add java packaging test project Adds a project for building and running packaging tests written in java for portability. The vagrant tasks use jars on the packagingTest configuration, which are built in the same project. No tests are added yet. Corresponding changes are not made to :x-pack:qa:vagrant because the java packaging tests will all be consolidated into one project. For #26741
Adds a project for building and running packaging tests written in java
for portability. The vagrant tasks use jars on the packagingTest
configuration, which are built in the same project. No tests are added
yet.
Corresponding changes are not made to :x-pack:qa:vagrant because the
java packaging tests will all be consolidated into one project.
For #26741