-
Notifications
You must be signed in to change notification settings - Fork 27
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(test): writing integration tests #430
base: main
Are you sure you want to change the base?
docs(test): writing integration tests #430
Conversation
Why was this closed? I don't see why we wouldn't benefit from having clearer documentation on how to write integration tests. |
🤔 I was pruning stale branches in my fork, and completely forgot about this PR... I'll re-open. |
c75c53f
to
e532a76
Compare
e532a76
to
78e3fa9
Compare
|
||
It's a good idea to run the whole cachi2 integration test suite, just to make | ||
sure everything still works properly. The command for this, *from inside the | ||
cachi2 repo* is `tox -e integration`. You can also provide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also want to mention the make test-integration
shortcut or would that only add noise?
executing e.g. | ||
|
||
```bash | ||
podman run --rm -ti -v "~/temp/cachi2-test:~/temp/cachi2-test:z" -w "~/temp/cachi2-test" localhost/cachi2:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need -it
if we're only executing commands with cachi2 rather than trying to run an interactive session (which also would not work without overriding the entry point)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copypasta from somewhere... I'll change it.
It's a good idea to run the whole cachi2 integration test suite, just to make | ||
sure everything still works properly. The command for this, *from inside the | ||
cachi2 repo* is `tox -e integration`. You can also provide | ||
`CACHI2_IMAGE=localhost/cachi2:latest` if you already have a current cachi2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good you mention this, but it feels weird that you actually explain the variable after the walkthrough section (next commit) which comes before this one.
|
||
At this point, you should be able to test locally. | ||
|
||
## Running the test suite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need another section to describe some general good practices when it comes to running our integration test suite, the contents can live happily in the previous one IMO.
1. Now run | ||
|
||
```bash | ||
CACHI2_IMAGE=localhost/cachi2:latest pytest -rA -vvvv --confcutdir=tests/integration --log-cli-level=DEBUG tests/integration/test_foo.py::test_foo_package[foo_incorrect_checksum] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running with an already built image IMO only makes sense when your debugging an issue, but from a developer's perspective you just added some code to cachi2 that you may have tested locally outside container environment, so you actually want the image to be rebuilt for tests. By all means we have to document the env variable, I just don't think that what you're proposing in the guideline is going to be the case for most invocations during test development.
Once you have working test sources, you'll need to commit and push them | ||
somewhere that pytest can clone them from. | ||
|
||
We *strongly* recommend making a fork, specific to your test, from one of the | ||
repos found under the [cachito test repos][] GitHub org (note that there | ||
are *many* repos there, covering all of the package managers which cachi2 and | ||
Cachito support) [^1]. | ||
|
||
Once you have a fork, push to a new branch named after your scenario - now | ||
pytest will have a proper commit hash in a repo to which you have complete | ||
access (once your test is complete and passing, you can simply open a PR against | ||
the repo in the [cachito-testing][] org). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is summarized later in the walkthrough in a step-by-step fashion. Can we merge this into the walkthrough so that it doesn't feel like repeating the same information with slightly different wording?
78e3fa9
to
76339c0
Compare
also describes the local development process: - processing test data with cachi2 - running pytest Signed-off-by: Ben Alkov <[email protected]>
To be squashed after review
76339c0
to
d89a07f
Compare
@ben-alkov do you plan on re-spinning this? |
Creating as a draft cuz it's a little rough, but I wanted to make sure I captured the essence and the specific commands while they were fresh in my head. Everything's on the table! Let me know if there's anything you'd like to do differently.
also describes the local development process:
Signed-off-by: Ben Alkov [email protected]
Maintainers will complete the following section
Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:
/ok-to-test
(as is the standard for Pipelines as Code)