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

[test] package pre-install java check #32259

Merged

Conversation

andyb-elastic
Copy link
Contributor

This recreates a test that was added to the bats packaging tests
in #31343 but didn't make it over to the java project during when the
linux package tests were ported in #31943

When packages are installed but can not locate the java executable, they
should fail with a descriptive message

This recreates a test that was added to the bats packaging tests
in elastic#31343 but didn't make it over to the java project during when the
linux package tests were ported in elastic#31943

When packages are installed but can not locate the java executable, they
should fail with a descriptive message
@andyb-elastic andyb-elastic added >test Issues or PRs that are addressing/adding tests review :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v7.0.0 v6.4.0 labels Jul 20, 2018
@andyb-elastic andyb-elastic requested a review from alpar-t July 20, 2018 23:31
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

LGTM

Platforms.onDPKG(() -> sh.run("dpkg -i " + distributionFile));

return Installation.ofPackage(distribution.packaging);
if (Platforms.isRPM()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the more I see of this, the more I think that it's worth extending the onDPKG type of API to be able to produce values so reading trough the code is more consistent.

Copy link
Contributor Author

@andyb-elastic andyb-elastic Jul 23, 2018

Choose a reason for hiding this comment

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

Thanks. The version of that I tried looked like this

Result result = Platforms.onRpm("...")
if (result == null) {
  result = Platforms.onDpkg("...")
}
return result;

because the onFoo methods still have to return something when they don't run their action. Which doesn't seem much more readable to me. Doing it with an Optional is a little better. Is there a better one that I'm missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

   Result result = Platforms.packagingChoice()
        .onRpm("...")
        .onDpkg()
        .get()

The packagingChoice() is optional, only if you don't want to keep the Platforms class growing.
The onRpm() etc type of methods will an instance, say ResultTracker<Result> and depending of the choice to be made will or will not add a result to it. get() will just return the result or throw an exception. We could also have a ResultTracker<Void> and a decide() method instead of get() that doesn't return anything. Maybe PackagingDecisionMaker is a better name than ResultTracker. Since this is for testing, we could have all the result trackers/decision makers register into some static context in Platforms perhaps, and add a check after each test to make sure that all of them are "decided" to make sure that one doesn't Platforms.onmRpm().onDpkg() without forgetting to call decide().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right we had something similar in a discussion on another PR, thanks. I'll likely open another one to make that change since we have a few places that would benefit from it now

@andyb-elastic andyb-elastic merged commit ac04ba4 into elastic:master Jul 23, 2018
andyb-elastic added a commit to andyb-elastic/elasticsearch that referenced this pull request Jul 24, 2018
This recreates a test that was added to the bats packaging tests
in elastic#31343 but didn't make it over to the java project during when the
linux package tests were ported in elastic#31943

When packages are installed but can not locate the java executable, they
should fail with a descriptive message
dnhatn added a commit that referenced this pull request Jul 25, 2018
* master:
  Security: revert to old way of merging automata (#32254)
  Networking: Fix test leaking buffer (#32296)
  Undo a debugging change that snuck in during the field aliases merge.
  Painless: Update More Methods to New Naming Scheme (#32305)
  [TEST] Fix assumeFalse -> assumeTrue in SSLReloadIntegTests
  Ingest: Support integer and long hex values in convert (#32213)
  Introduce fips_mode setting and associated checks (#32326)
  Add V_6_3_3 version constant
  [DOCS] Removed extraneous callout number.
  Rest HL client: Add put license action (#32214)
  Add ERR to ranking evaluation documentation (#32314)
  Introduce Application Privileges with support for Kibana RBAC (#32309)
  Build: Shadow x-pack:protocol into x-pack:plugin:core (#32240)
  [Kerberos] Add Kerberos authentication support (#32263)
  [ML] Extract persistent task methods from MlMetadata (#32319)
  Add Restore Snapshot High Level REST API
  Register ERR metric with NamedXContentRegistry (#32320)
  fixes broken build for third-party-tests (#32315)
  Allow Integ Tests to run in a FIPS-140 JVM (#31989)
  [DOCS] Rollup Caps API incorrectly mentions GET Jobs API (#32280)
  awaitsfix testRandomClusterStateUpdates
  [TEST] add version skip to weighted_avg tests
  Consistent encoder names (#29492)
  Add WeightedAvg metric aggregation (#31037)
  Switch monitoring to new style Requests (#32255)
  Rename ranking evaluation `quality_level` to `metric_score` (#32168)
  Fix a test bug around nested aggregations and field aliases. (#32287)
  Add new permission for JDK11 to load JAAS libraries (#32132)
  Silence SSL reload test that fails on JDK 11
  [test] package pre-install java check (#32259)
  specify subdirs of lib, bin, modules in package (#32253)
  Switch x-pack:core to new style Requests (#32252)
  awaitsfix SSLConfigurationReloaderTests
  Painless: Clean up add methods in PainlessLookup (#32258)
  Fail shard if IndexShard#storeStats runs into an IOException (#32241)
  AwaitsFix RecoveryIT#testHistoryUUIDIsGenerated
  Remove unnecessary warning supressions (#32250)
  CCE when re-throwing "shard not available" exception in TransportShardMultiGetAction (#32185)
  Add new fields to monitoring template for Beats state (#32085)
andyb-elastic added a commit that referenced this pull request Jul 25, 2018
This recreates a test that was added to the bats packaging tests
in #31343 but didn't make it over to the java project during when the
linux package tests were ported in #31943

When packages are installed but can not locate the java executable, they
should fail with a descriptive message
dnhatn added a commit that referenced this pull request Jul 27, 2018
* 6.x:
  Only enforce password hashing check if FIPS enabled (#32383)
  Introduce fips_mode setting and associated checks (#32326)
  [DOCS] Fix formatting error in Slack action
  Ingest: Support integer and long hex values in convert (#32213)
  Release pipelined request in netty server tests (#32368)
  Add opaque_id to index audit logging (#32260)
  Painless: Fix documentation links to use existing refs (#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (#32346)
  [DOCS] Adds recommendation for xpack.security.enabled (#32345)
  [test] package pre-install java check (#32259)
  [DOCS] Adds link from bucket_span property to common time units
  [DOCS] Fixes typo in ML aggregations page
  [ML][DOCS] Add documentation for detector rules and filters (#32013)
  Bump the 6.x branch to 6.5.0 (#32361)
  fixes broken build repository-s3 for third-party-tests
@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 Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants