Skip to content
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

[docs] explainer for java packaging tests #30825

Merged

Conversation

andyb-elastic
Copy link
Contributor

Follow up from #30734

@andyb-elastic andyb-elastic added the >docs General docs changes label May 23, 2018

## Running these tests

See the section in [TESTING.asciidoc](../../TESTING.asciidoc#testing-packaging)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move that section into here and link from there to here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, what do you think @rjernst

## Adding a new test class

When gradle runs the packaging tests on a VM, it runs the full suite by
default. To add a test class to the suite, add its `class` to the
Copy link
Member

Choose a reason for hiding this comment

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

Does it run them in the order that they are declared? Maybe put that if it is true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, I'll add that

@andyb-elastic
Copy link
Contributor Author

Jenkins run gradle build tests please

Add note about suite test ordering
@andyb-elastic andyb-elastic added the :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts label May 23, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-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. I left a couple comments, most of which can be followups, especially since some will require refactoring the actual code.


When gradle runs the packaging tests on a VM, it runs the full suite by
default. To add a test class to the suite, add its `class` to the
`@SuiteClasses` annotation in [PackagingTests.java](src/main/java/org/elasticsearch/packaging/PackagingTests.java).
Copy link
Member

Choose a reason for hiding this comment

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

I had not noticed this requirement before when reviewing. I think this will be very error prone, and we should change this in a followup. Randomized runner automatically finds the suites to invoke (iirc the ant runner is what finds the classes). I think we can do similar from gradle in generating the test shell script that invokes the test runner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'm not sure if there's anything in junit 4 that does this out of the box but it shouldn't be too hard to get working

you can make an assumption using the constants and methods in [Platforms.java](src/main/java/org/elasticsearch/packaging/util/Platforms.java)

```java
assumeThat("only run on windows", Platforms.WINDOWS, is(true));
Copy link
Member

Choose a reason for hiding this comment

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

I would write this with assumeTrue, since the constant is already a boolean...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I'll do that in this PR since it's a small change

Java alternative exists. For example most filesystem operations can be done with
the java.nio.file APIs. For those that aren't, use an instance of [Shell](src/main/java/org/elasticsearch/packaging/util/Shell.java)

Despite the name, commands run with this class are not run in a shell, and any
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason it couldn't be run in a shell? The Shell ctor could take the shell to run, ie bash or powershell? Otherwise I think we should rename Shell to Command or something similar, as it it would be too confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense. Running commands in bash is probably less likely to have weird issues, and we'll be wanting to run all the windows stuff in powershell anyway. I'll make a follow up for this

@andyb-elastic
Copy link
Contributor Author

Jenkins run full packaging tests please

@andyb-elastic
Copy link
Contributor Author

Whoops, Jenkins run all packaging tests please

@andyb-elastic andyb-elastic merged commit 4bd2607 into elastic:master May 25, 2018
martijnvg added a commit that referenced this pull request May 25, 2018
* es/master:
  Move score script context from SearchScript to its own class (#30816)
  Fix bad version check writing Repository nodes (#30846)
  [docs] explainer for java packaging tests (#30825)
  Remove Throwable usage from transport modules (#30845)
  REST high-level client: add put ingest pipeline API (#30793)
  Update the version checks around ip_range bucket keys, now that the change was backported.
  Mute IndexMasterFailoverIT.testMasterFailoverDuringIndexingWithMappingChanges
  Use geohash cell instead of just a corner in geo_bounding_box (#30698)
  Limit user to single concurrent auth per realm (#30794)
  [Tests] Move templated _rank_eval tests (#30679)
  Security: fix dynamic mapping updates with aliases (#30787)
  Ensure that ip_range aggregations always return bucket keys. (#30701)
  Use remote client in TransportFieldCapsAction (#30838)
  Move Watcher versioning setting to meta field (#30832)
  [Docs] Explain incomplete dates in range queries (#30689)
  Move persistent task registrations to core (#30755)
  Decouple ClusterStateTaskListener & ClusterApplier (#30809)
  Send client headers from TransportClient (#30803)
  Packaging: Ensure upgrade_is_oss flag file is always deleted (#30732)
  Force stable file modes for built packages (#30823)
dnhatn added a commit that referenced this pull request May 28, 2018
* 6.x:
  Fix double semicolon in import statement
  [TEST] Fix minor random bug from #30794
  Enabling testing against an external cluster (#30885)
  SQL: Remove the last remaining server dependencies from jdbc (#30771)
  Add public key header/footer (#30877)
  Include size of snapshot in snapshot metadata (#29602)
  QA: Test template creation during rolling restart (#30850)
  REST high-level client: add put ingest pipeline API (#30793)
  Do not serialize basic license exp in x-pack info (#30848)
  [docs] explainer for java packaging tests (#30825)
  Verify signatures on official plugins (#30800)
  [DOCS] Document index name limitations (#30826)
  [Docs] Add reindex.remote.whitelist example (#30828)
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >docs General docs changes Team:Delivery Meta label for Delivery team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants