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: Add test file location -> test runner table #34986

Merged
merged 6 commits into from Apr 19, 2019

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Apr 12, 2019

Summary

Following discussions around #34557 this adds documentation to CONTRIBUTING.md to try to address the following pain point:

On a bad day my workflow looks like this:

  1. Make some changes
  2. Try to test only the affected changes.
  3. Give up and push to CI
  4. Find something else to do while I wait for test results (~40mins)
  5. Repeat

To improve this, I'd like to be able to:

  1. Easily find out how to run a specific test file so that I can quickly run the tests most likely to be affected by my change (or even better, have a single test command that takes the file I'd like to test as argument).
  2. If CI fails I want to know how to run the failing test cases locally so that I can have a short feedback loop when addressing these.

Please add suggestions for further test locations/runner invocations if there's anything I missed.

I wasn't able to figure out the following:

  1. How to identify from a file location if a test might be a karma test or how to run that specific file.
  2. How to run functional tests if the config file is a typescript file

The following table outlines possible test file locations and how to invoke them:

Test location Runner command (working directory is kibana root)
src/**/*.test.js
src/**/*.test.ts
yarn test:jest -t regexp [test path]
src/**/__tests__/**/*.js
packages/kbn-datemath/test/**/*.js
packages/kbn-dev-utils/src/**/__tests__/**/*.js
tasks/**/__tests__/**/*.js
node scripts/mocha --grep=regexp [test path]
**/integration_tests/**/*.test.js node scripts/jest_integration -t regexp [test path]
test/*integration/**/config.js
test/*functional/**/config.js
x-pack/test/*integration/**/config.js
x-pack/test/*functional/config.js
node scripts/functional_tests --config test/[directory]/config.js --grep=regexp
x-pack/**/*.test.js
x-pack/**/*.test.ts
cd x-pack && yarn test:jest -t regexp [test path]
x-pack/test/*integration/**/config.ts ??

Test runner arguments:

  • Where applicable, the optional arguments -t=regexp or --grep=regexp will only run tests or test suites whose descriptions matches the regular expression.
  • [test path] is the relative path to the test file.

Examples:

  • Run the entire elasticsearch_service test suite with yarn:
    yarn test:jest src/core/server/elasticsearch/elasticsearch_service.test.ts
  • Run the jest test case whose description matches 'stops both admin and data clients':
    yarn test:jest -t 'stops both admin and data clients' src/core/server/elasticsearch/elasticsearch_service.test.ts
  • Run the x-pack api integration test case whose description matches the given string:
    node scripts/functional_tests --config x-pack/test/api_integration/config.js --grep='apis Monitoring Beats list with restarted beat instance should load multiple clusters'

@rudolf rudolf requested a review from a team April 12, 2019 13:25
@rudolf rudolf added Team:Operations Team label for Operations Team review v7.2.0 v8.0.0 labels Apr 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

CONTRIBUTING.md Outdated
| `**/integration_tests/**/*.test.js` | `node scripts/jest_integration -t regexp [test path]` |
| `test/*integration/**/config.js`<br>`test/*functional/**/config.js`<br>`x-pack/test/*integration/**/config.js`<br>`x-pack/test/*functional/config.js` | `node scripts/functional_tests --config test/[directory]/config.js --grep=regexp` |
| `x-pack/**/*.test.js`<br>`x-pack/**/*.test.ts` | `cd x-pack && yarn test:jest -t regexp [test path]` |
| `x-pack/test/*integration/**/config.ts` | ?? |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the value for this row can be:

cd x-pack && node scripts/functional_tests_server.js
node scripts/functional_test_runner.js --config x-pack/test/api_integration/config.js --grep "Some string from the test case name"

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to keep the X-Pack specific docs here: https://github.com/elastic/kibana/blob/master/x-pack/README.md until we unify all of the tooling.

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM. It would be nice to cross-link between those two docs, in that case.

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 problem I ran into here was how do I run functional tests if the config file is written as a typescript file as opposed to a javascript file?

CONTRIBUTING.md Outdated

The following table outlines possible test file locations and how to invoke them:

| Test location | Runner command (working directory is kibana root) |
Copy link
Contributor

Choose a reason for hiding this comment

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

how about adding a column at the beginning of the table called “Type”? Values for each row could be “Jest”, “Mocha”, “Jest + API”, “Functional”, “Jest (X-Pack”), and “Integration (X-Pack)”

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having the rows be by type would help simplify this quite a bit. They would then correspond to the locations they cover and the command to execute.

@rudolf rudolf mentioned this pull request Apr 15, 2019
@rudolf
Copy link
Contributor Author

rudolf commented Apr 16, 2019

@cjcenizal @tylersmalley I played with grouping tests by "type" and splitting x-pack tests out of this file (just pasting as a comment, didn't push any code yet). It felt weird to group integration and functional tests under the "functional" type, wouldn't it be two distinct types "functional" and "integration"?

Type Test location Runner command (working directory is kibana root)
Jest src/**/*.test.js
src/**/*.test.ts
yarn test:jest -t regexp [test path]
Mocha src/**/__tests__/**/*.js
packages/kbn-datemath/test/**/*.js
packages/kbn-dev-utils/src/**/__tests__/**/*.js
tasks/**/__tests__/**/*.js
node scripts/mocha --grep=regexp [test path]
Jest Integration **/integration_tests/**/*.test.js node scripts/jest_integration -t regexp [test path]
Functional test/*functional/**/config.js node scripts/functional_tests --config test/[directory]/config.js --grep=regexp
Integration test/*integration/**/config.js node scripts/functional_tests --config test/[directory]/config.js --grep=regexp
Type Test location Runner command (working directory is kibana root)
X-Pack Jest x-pack/**/*.test.js
x-pack/**/*.test.ts
cd x-pack && yarn test:jest -t regexp [test path]
X-Pack Integration x-pack/test/*integration/**/config.js cd x-pack && node scripts/functional_tests --config test/[directory]/config.js --grep=regexp
X-Pack Functional x-pack/test/*functional/config.js cd x-pack && node scripts/functional_tests --config test/[directory]/config.js --grep=regexp

But then I wondered if it would be simpler to group it under the test runner, in which case both functional and integration tests uses the "functional test runner":

Test runner Test location Runner command (working directory is kibana root)
Jest src/**/*.test.js
src/**/*.test.ts
yarn test:jest -t regexp [test path]
Jest (integration) **/integration_tests/**/*.test.js node scripts/jest_integration -t regexp [test path]
Mocha src/**/__tests__/**/*.js
packages/kbn-datemath/test/**/*.js
packages/kbn-dev-utils/src/**/__tests__/**/*.js
tasks/**/__tests__/**/*.js
node scripts/mocha --grep=regexp [test path]
Functional test/*integration/**/config.js
test/*functional/**/config.js
node scripts/functional_tests --config test/[directory]/config.js --grep=regexp
Test runner Test location Runner command (working directory is kibana root)
Jest x-pack/**/*.test.js
x-pack/**/*.test.ts
cd x-pack && yarn test:jest -t regexp [test path]
Functional x-pack/test/*integration/**/config.js
x-pack/test/*functional/config.js
cd x-pack && node scripts/functional_tests --config test/[directory]/config.js --grep=regexp

@rudolf rudolf requested review from tylersmalley and cjcenizal and removed request for tylersmalley April 16, 2019 13:36
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

Just a couple small changes - thanks for your work on this!

CONTRIBUTING.md Outdated

| Test runner | Test location | Runner command (working directory is kibana root) |
| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- |
| Jest | `src/**/*.test.js`<br>`src/**/*.test.ts` | `yarn test:jest -t regexp [test path]` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we update this to node scripts/jest

CONTRIBUTING.md Outdated
| Jest | `src/**/*.test.js`<br>`src/**/*.test.ts` | `yarn test:jest -t regexp [test path]` |
| Jest (integration) | `**/integration_tests/**/*.test.js` | `node scripts/jest_integration -t regexp [test path]` |
| Mocha | `src/**/__tests__/**/*.js`<br>`packages/kbn-datemath/test/**/*.js`<br>`packages/kbn-dev-utils/src/**/__tests__/**/*.js`<br>`tasks/**/__tests__/**/*.js` | `node scripts/mocha --grep=regexp [test path]` |
| Functional | `test/*integration/**/config.js`<br>`test/*functional/**/config.js` | `node scripts/functional_tests --config test/[directory]/config.js --grep=regexp` |
Copy link
Contributor

Choose a reason for hiding this comment

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

While this will run the tests, I am concerned folks will use this for debugging which takes a large amount of time to start Kibana/ES each time. We have details here regarding how to run for development: https://www.elastic.co/guide/en/kibana/current/development-functional-tests.html#_running_functional_tests

I would suggest we update this command to outline the two:

  • node scripts/functional_tests_server --config test/[directory]/config.js
  • node scripts/functional_test_runner --config test/[directory]/config.js --grep=regexp

x-pack/README.md Outdated
## Running specific tests
| Test runner | Test location | Runner command (working directory is kibana/x-pack) |
| ------------ | ----------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- |
| Jest | `x-pack/**/*.test.js`<br>`x-pack/**/*.test.ts` | `cd x-pack && yarn test:jest -t regexp [test path]` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Same feedback as for Kibana - use scripts/jest and outline the separate server/runner functional test scripts.

CONTRIBUTING.md Outdated
`yarn test:jest src/core/server/elasticsearch/elasticsearch_service.test.ts`
- Run the jest test case whose description matches 'stops both admin and data clients':
`yarn test:jest -t 'stops both admin and data clients' src/core/server/elasticsearch/elasticsearch_service.test.ts`
- Run the x-pack api integration test case whose description matches the given string:
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this X-Pack example since you included in the X-Pack docs

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

💯 This is fantastic! Thanks so much for doing this.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rudolf rudolf merged commit bb121ef into elastic:master Apr 19, 2019
rudolf added a commit that referenced this pull request Apr 19, 2019
* docs: Add test file location -> test runner table

* Use scripts/mocha instead of if yarn test:mocha and clarify arguments

* docs: add some examples of running tests suites with grep patterns

* Split out x-pack testing docs, add test runner column

* Split out x-pack testing docs, add test runner column #2

* Use node scripts for jest and split functional runner/server
rudolf added a commit to rudolf/kibana that referenced this pull request Apr 19, 2019
* docs: Add test file location -> test runner table

* Use scripts/mocha instead of if yarn test:mocha and clarify arguments

* docs: add some examples of running tests suites with grep patterns

* Split out x-pack testing docs, add test runner column

* Split out x-pack testing docs, add test runner column #2

* Use node scripts for jest and split functional runner/server
@rudolf rudolf deleted the test-runner-docs branch April 19, 2019 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Operations Team label for Operations Team v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants