Add Makefile target to run all CI checks #1299
Conversation
And reorganize the docs to mention integration tests and this new target.
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| @@ -8,6 +8,6 @@ attention. | |||
|
|
|||
| * Have you signed the [contributor license agreement](https://www.elastic.co/contributor-agreement)? | |||
| * Have you followed the [contributor guidelines](https://github.com/elastic/rally/blob/master/CONTRIBUTING.md)? | |||
| * Have you run `make test` and `make it` and both finish without errors? | |||
| * Have you run `make lint`, `make test` and `make it` and they finish without errors? | |||
There was a problem hiding this comment.
Regarding your PR comment we want to ensure all tests pass before raising a community PR is raised so I suggest we replace this with Have you run `make check-all` successfully? This reduces time spent in the PR review process.
CONTRIBUTING.md
Outdated
|
|
||
| Consider using editor integrations to do it automatically: you'll need to configure [black](https://black.readthedocs.io/en/stable/integrations/editors.html) and [isort](https://github.com/PyCQA/isort/wiki/isort-Plugins). | ||
| Run the test suite to make sure that nothing is broken: `make test`. You can also run integration tests with `make it` (they're much slower, though). To run everything including lint, use `make check-all`. |
There was a problem hiding this comment.
I suggest we change the order here and require that make check-all passes first -- since these are the guidelines before raising a PR -- and then explain that to iterate faster make test and make it can also be used. In other words something like:
Ensure that all tests pass by running
make check-all. This runs sequentially lint checks, unit tests and integration tests. These can be executed in isolation usingmake lint,make testandmake itrespectively, in case you need to iterate over a subset of tests.Note: Integration tests are much slower than unit tests.
dliappis
added
:Docs
|
@elasticmachine test this please |
|
@elasticmachine test this please |
|
CI is green :) |
|
@pquentin Ah sorry, yes this fell through the cracks. Merging. |
And reorganize the docs to mention integration tests and this new target.
I also noticed an opportunity to update the pull request template. I think I prefer not mentioning
make check-allthere because I still haven't managed to run the integration tests locally!Closes #1297