Skip to content

[Functional Tests] Use @kbn/test on Kibana CI#18967

Merged
rhoboat merged 22 commits intoelastic:masterfrom
rhoboat:archanid-18593
Jun 26, 2018
Merged

[Functional Tests] Use @kbn/test on Kibana CI#18967
rhoboat merged 22 commits intoelastic:masterfrom
rhoboat:archanid-18593

Conversation

@rhoboat
Copy link

@rhoboat rhoboat commented May 10, 2018

Addresses #18593

Background

We currently use the @kbn/test package on CI in one place: running X-Pack Functional, API, and SAML API tests. These run in succession with a single script (node scripts/functional_tests from x-pack/).

We'd like to use the package to run OSS Kibana tests on CI as well. Currently we run API tests in the test:api grunt task, and Selenium (functional) tests in the jenkins:selenium grunt task. We can unify these or keep them separate. (See #18593 for high-level details.)

Running from source/snapshot

One of the key requirements:

ES should always run from source when on CI. It should run from snapshot by default (for dev).

Now pass in --esFrom=<source|snapshot> default: snapshot

  • Check for invalid cli args
  • Replace api tests
  • Replace selenium tests
  • Jest tests

How to test:

Unit tests

There are Jest tests.

Integration tests

Look at the test run outputs in Jenkins.

  • Search for Running "run:functional_tests" (run) task in multijob-selenium task
  • Search for Running "run:api_integration_tests" (run) task in multijob-intake task

Then in the console output, just take a look at how Elasticsearch is started up (from source on CI), how Kibana is run with cli args, and how functional test runner runs.

Manual testing in development

# bail option not a boolean
node scripts/functional_tests --config test/functional/config.js --esFrom snapshot --debug --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# esFrom option not valid
node scripts/functional_tests --config test/functional/config.js --esFrom butter --debug --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# config option not a string
node scripts/functional_tests --config --esFrom source --debug --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# works with esFrom = source
node scripts/functional_tests --config test/functional/config.js --esFrom source --debug --grep management --bail -- --server.maxPayloadBytes=1648576

# server.<option> options not valid before the --
node scripts/functional_tests --config test/functional/config.js --esFrom source --debug --server.maxPayloadBytes=1648576 --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# works with multiple config options
node scripts/functional_tests --config test/functional/config.js --config test/api_integration/config.js --esFrom snapshot --debug --grep management --bail -- --server.maxPayloadBytes=1648576

# works with no config options
node scripts/functional_tests --esFrom snapshot --debug --grep management --bail -- --server.maxPayloadBytes=1648576

@rhoboat rhoboat changed the title Replace test:api with @kbn/test runTests [Functional Tests] Use @kbn/test on Kibana CI May 10, 2018
Copy link
Author

@rhoboat rhoboat May 10, 2018

Choose a reason for hiding this comment

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

  • Remove test:api related code.
  • Replace it with a call to @kbn/test code, scoped to just run api tests.
  • Run api_integration_tests using the scripts/functional_tests (which uses @kbn/test
    • Pass in just the config for api_integration tests
    • jenkins_config.js is built on the original with just a change to run ES from source

Alternative way to do this:

  • Call runTests directly from @kbn/test, though I'm not sure how to do this. If it needs to be done in a command-line way, then we already have it in scripts/functional_tests.
  • Allow test/api_integration/config.js to take in a from.
  • Or allow scripts/functional_tests to take in a --from.
    • This approach seems worth trying, for comparison.

@rhoboat
Copy link
Author

rhoboat commented May 10, 2018

@tylersmalley @azasypkin @spalger @epixa What do you think so far? I.e. Which of the 3 alternatives would you prefer?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link
Author

rhoboat commented May 10, 2018

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger
Copy link
Contributor

spalger commented May 10, 2018

What about a fourth option, the CI environment variable is always set when we run in CI, so we could just update https://github.com/elastic/kibana/blob/master/test/common/config.js#L21

    esTestCluster: {
      license: 'oss',
-     from: 'snapshot',
+     from: process.env.CI ? 'source' : 'snapshot',
      serverArgs: [
      ],
    },

@spalger
Copy link
Contributor

spalger commented May 10, 2018

Actually, relying on the CI environment variable makes it pretty tricky to run elasticsearch from source intentionally, and will require a good amount of digging into the code to figure out how.

I'm personally a fan of the --es-from command line flag, mostly because I like that it's documented in --help and that command line flags are normally how I tweak the behavior of a command line tool.

I just ran the x-pack/scripts/functional_tests_server script and was surprised by my inability to choose where es was built from. I tried --help, but it didn't offer any, and I would prefer that it did.

@tylersmalley
Copy link
Member

I am also +1 for the command line argument. Running from source is not something that we just want to do for Jenkins, it's also if you are working off an ES feature branch.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link
Author

rhoboat commented May 17, 2018

jenkins test this

@kimjoar kimjoar mentioned this pull request May 17, 2018
13 tasks
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@spalger spalger May 21, 2018

Choose a reason for hiding this comment

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

These changes seems really redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use options['es-from'] || 'snapshot'? This ternary will allow passing --es-from=foo and interpret it as --es-from=snapshot because any none "source" value is ignored.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link
Author

rhoboat commented May 21, 2018

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link
Author

rhoboat commented May 22, 2018

hmm. this keeps failing

23:50:16 │ proc [ftr] └- ✖ fail: "management test large number of fields test_huge data should have expected number of fields"
23:50:16 │ proc [ftr] │ tryForTime timeout: Error: tryForTime timeout: Error: tryForTime timeout: [POST http://localhost:9515/session/ba2732fe4fefc2e1444fba16e5f86da7/element / {"using":"css selector","value":"[data-test-subj~="tab-count-indexedFields"]"}] no such element: Unable to locate element: {"method":"css selector","selector":"[data-test-subj~="tab-count-indexedFields"]"}

@rhoboat
Copy link
Author

rhoboat commented May 22, 2018

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat rhoboat added the WIP Work in progress label May 22, 2018
@rhoboat
Copy link
Author

rhoboat commented Jun 20, 2018

@kobelb @azasypkin I've made the default log level debug for the tasks in tasks/config/run.js

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rhoboat
Copy link
Author

rhoboat commented Jun 20, 2018

@azasypkin yes, indeed, was waiting for that dev.mode flag removal. 🎉

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link
Author

rhoboat commented Jun 22, 2018

@kobelb I took care of your comments, i think you're out on vacation...

@rhoboat
Copy link
Author

rhoboat commented Jun 22, 2018

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rhoboat
Copy link
Author

rhoboat commented Jun 26, 2018

jenkins, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, looks great! I've run all possible functional/integration tests I could think of in Kibana and x-pack, everything worked flawlessly.

--help Display this menu and exit.
--config <file> Pass in a config. Can pass in multiple configs.
--esFrom <snapshot|source> Build Elasticsearch from source or run from snapshot. Default: snapshot
--kibana-install-dir <dir> Run Kibana from existing install directory instead of from source.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it's the only parameter that uses dashes now, maybe we can make it --kibanaInsallDir just for consistency?

Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

LGTM - the improved test coverage is great!

@rhoboat rhoboat merged commit b03243f into elastic:master Jun 26, 2018
mattapperson pushed a commit that referenced this pull request Jun 28, 2018
* Replace test:api with @kbn/test runTests

* Improve CLI help menu 🆘

* Use --es-from

* Replace jenkins:selenium with kbn-test

* Validate cli args, fixing test in the process

* Clean up some stuff

* Code review fixes

* Explanation for collectCliArgs

* Remove exit codes, they're useless anyway.

* Make markdown vis test pass with dev_mode setting

* Tests

* Remove unneeded export

* Code review: move console logging up to cli.js

* Code review: refactor startServers and runTests to take single options collection

* Code review: Remove all things I am sure we do not use

* Improve tests

* Code review fixes

* Pass created log to runFtr, runElasticsearch, runKibanaServer

* Update --es-from option to --esFrom
@rhoboat rhoboat deleted the archanid-18593 branch June 28, 2018 16:56
rhoboat pushed a commit that referenced this pull request Jun 29, 2018
* [Functional Tests] Use @kbn/test on Kibana CI (#18967)

* Replace test:api with @kbn/test runTests

* Improve CLI help menu 🆘

* Use --es-from

* Replace jenkins:selenium with kbn-test

* Validate cli args, fixing test in the process

* Clean up some stuff

* Code review fixes

* Explanation for collectCliArgs

* Remove exit codes, they're useless anyway.

* Make markdown vis test pass with dev_mode setting

* Tests

* Remove unneeded export

* Code review: move console logging up to cli.js

* Code review: refactor startServers and runTests to take single options collection

* Code review: Remove all things I am sure we do not use

* Improve tests

* Code review fixes

* Pass created log to runFtr, runElasticsearch, runKibanaServer

* Update --es-from option to --esFrom

* Let dev server run from snapshot by default
rhoboat pushed a commit that referenced this pull request Jun 29, 2018
* [Functional Tests] Use @kbn/test on Kibana CI (#18967)

* Replace test:api with @kbn/test runTests

* Improve CLI help menu 🆘

* Use --es-from

* Replace jenkins:selenium with kbn-test

* Validate cli args, fixing test in the process

* Clean up some stuff

* Code review fixes

* Explanation for collectCliArgs

* Remove exit codes, they're useless anyway.

* Make markdown vis test pass with dev_mode setting

* Tests

* Remove unneeded export

* Code review: move console logging up to cli.js

* Code review: refactor startServers and runTests to take single options collection

* Code review: Remove all things I am sure we do not use

* Improve tests

* Code review fixes

* Pass created log to runFtr, runElasticsearch, runKibanaServer

* Update --es-from option to --esFrom

* Still need dev_mode setting in 6.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants