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

[kbn-test] convert kibana-install-dir flag to installDir option #21317

Merged
merged 11 commits into from
Aug 3, 2018

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jul 26, 2018

Fixes #20922

Based on the variable that's extracted from the options object in https://github.com/elastic/kibana/blob/master/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js#L24 the --kibana-install-dir flag is expected to be the installDir option, but it's not being transformed as such by the cli/run_tests/args module. This updated the processOptions() function to convert the kibana-install-dir option to installDir, which should get the functional tests running against the build output again.

@spalger spalger added review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.0.0 v6.4.0 v6.5.0 labels Jul 26, 2018
@spalger spalger requested a review from rhoboat July 26, 2018 22:10
userOptions.installDir = userOptions['kibana-install-dir'];
delete userOptions['kibana-install-dir'];
}

Copy link

Choose a reason for hiding this comment

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

Copy link

@rhoboat rhoboat left a comment

Choose a reason for hiding this comment

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

Left a comment

@elastic elastic deleted a comment from elasticmachine Jul 26, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link

rhoboat commented Jul 27, 2018

retest

@spalger spalger force-pushed the fix/ftr/kibana-install-dir branch 3 times, most recently from ec533f5 to 37983ba Compare July 27, 2018 17:34
@spalger spalger force-pushed the fix/ftr/kibana-install-dir branch from 37983ba to ac0749b Compare July 27, 2018 17:35
@elastic elastic deleted a comment from elasticmachine Jul 27, 2018
spalger added 2 commits July 27, 2018 12:26
…eded

Adding a `webpack.DefinePlugin` slows down the optimizer a small amount,
so only apply it when it is necessary, and skip it if it is going to
be defined as "false".
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger
Copy link
Contributor Author

spalger commented Aug 1, 2018

@archanid this is ready for another look if you have a chance

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rhoboat
Copy link

rhoboat commented Aug 2, 2018

node scripts/functional_tests_server --config test/functional/config.js --kibana-install-dir ./build/kibana

gives me Error: Cannot find module '../../package.json'. It's looking for package.json in the build/kibana directory. Locally I don't have that. Is it created in the build?

@rhoboat
Copy link

rhoboat commented Aug 2, 2018

How to test, to save people time:

  1. run node scripts/kibana
  2. extract build from target/ dir
  3. try node scripts/functional_tests_server --kibana-install-dir ./path/to/extracted/build
  4. make sure kibana starts up from build

@@ -76,7 +76,7 @@ export async function startServers(options) {
config,
options: {
...opts,
extraKbnOpts: [...options.extraKbnOpts, '--dev'],
extraKbnOpts: [...options.extraKbnOpts, ...(options.installDir ? [] : ['--dev'])],
Copy link

Choose a reason for hiding this comment

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

Is there an explanation for this? I'm curious...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The distributable doesn't support the --dev flag, it's only available when running from source.

@@ -18,7 +18,7 @@
*/

require('../src/setup_node_env');
require('../packages/kbn-test').runTestsCli([
require('@kbn/test').runTestsCli([
Copy link

Choose a reason for hiding this comment

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

++

@@ -30,7 +30,7 @@ export class KibanaServerVersion {

const status = await this.kibanaStatus.get();
if (status && status.version && status.version.number) {
this._cachedVersionNumber = status.version.number;
this._cachedVersionNumber = status.version.number + (status.version.build_snapshot ? '-SNAPSHOT' : '');
Copy link

Choose a reason for hiding this comment

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

What's the reason for this change, also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this change just fixes the kibanaServer.version.get() method to properly mirror the version information used in the Kibana server by relying on the build_snapshot property in the API response.

This was necessary because the RBAC tests are the only thing relying on this method right now, and they were added while we weren't testing against the distributable, so they started failing when we fixed the --kibana-install-dir flag.

Copy link

@rhoboat rhoboat left a comment

Choose a reason for hiding this comment

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

Works fine. Just some questions about two things in the code changes.

@spalger spalger merged commit 4f0d2ad into elastic:master Aug 3, 2018
spalger pushed a commit to spalger/kibana that referenced this pull request Aug 3, 2018
…tic#21317)

* [kbn-test] convert kibana-install-dir flag to installDir option

* [kbn-test] replicate kibana-install-dir handling to startServers

* [ftr] try running functional tests in production in CI

* Revert "[ftr] try running functional tests in production in CI"

This reverts commit e5b94aa.

* [core/public/legacyPlatform] exclude ui/test_harness from the distributable

* [optimizer] fix `process.env.IS_KIBANA_DISTRIBUTABLE` definition

* [optimizer] only define `process.env.IS_KIBANA_DISTRIBUTABLE` when needed

Adding a `webpack.DefinePlugin` slows down the optimizer a small amount,
so only apply it when it is necessary, and skip it if it is going to
be defined as "false".

* [kbn-test/startServer] don't run in --dev mode if running from dist

* [ftr/kibanaServer/version] attach `-SNAPSHOT` suffix to version if running build_snapshot
spalger pushed a commit to spalger/kibana that referenced this pull request Aug 3, 2018
…tic#21317)

* [kbn-test] convert kibana-install-dir flag to installDir option

* [kbn-test] replicate kibana-install-dir handling to startServers

* [ftr] try running functional tests in production in CI

* Revert "[ftr] try running functional tests in production in CI"

This reverts commit e5b94aa.

* [core/public/legacyPlatform] exclude ui/test_harness from the distributable

* [optimizer] fix `process.env.IS_KIBANA_DISTRIBUTABLE` definition

* [optimizer] only define `process.env.IS_KIBANA_DISTRIBUTABLE` when needed

Adding a `webpack.DefinePlugin` slows down the optimizer a small amount,
so only apply it when it is necessary, and skip it if it is going to
be defined as "false".

* [kbn-test/startServer] don't run in --dev mode if running from dist

* [ftr/kibanaServer/version] attach `-SNAPSHOT` suffix to version if running build_snapshot
spalger pushed a commit that referenced this pull request Aug 3, 2018
…) (#21626)

* [kbn-test] convert kibana-install-dir flag to installDir option

* [kbn-test] replicate kibana-install-dir handling to startServers

* [ftr] try running functional tests in production in CI

* Revert "[ftr] try running functional tests in production in CI"

This reverts commit e5b94aa.

* [core/public/legacyPlatform] exclude ui/test_harness from the distributable

* [optimizer] fix `process.env.IS_KIBANA_DISTRIBUTABLE` definition

* [optimizer] only define `process.env.IS_KIBANA_DISTRIBUTABLE` when needed

Adding a `webpack.DefinePlugin` slows down the optimizer a small amount,
so only apply it when it is necessary, and skip it if it is going to
be defined as "false".

* [kbn-test/startServer] don't run in --dev mode if running from dist

* [ftr/kibanaServer/version] attach `-SNAPSHOT` suffix to version if running build_snapshot
spalger pushed a commit that referenced this pull request Aug 3, 2018
…#21317) (#21627)

Backports the following commits to 6.4:
 - [kbn-test] convert kibana-install-dir flag to installDir option  (#21317)
@spalger
Copy link
Contributor Author

spalger commented Aug 3, 2018

6.x/6.5: 0f12ecf
6.4: c937beb

@spalger spalger deleted the fix/ftr/kibana-install-dir branch August 3, 2018 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.4.0 v6.5.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants