Makefile: improve make help and add testing guide#28946
Conversation
|
PTAL @podman-container-tools/podman-maintainers @podman-container-tools/podman-reviewers |
| @@ -0,0 +1,86 @@ | |||
| # Running Podman Tests On Linux | |||
There was a problem hiding this comment.
Don't we already have a testing readme? test/README.md?
There was a problem hiding this comment.
Yes, but it's missing a few useful things. My idea was to have a single file with just commands and brief notes, but it would probably be better to extend test/README.md instead.
| - See `test/README.md` for additional details on test infrastructure. | ||
|
|
||
| ## Quick Reference | ||
|
|
||
| | Command | What it does | | ||
| |---------|-------------| | ||
| | `make localunit` | Unit tests with coverage | | ||
| | `make localintegration` | Integration tests (local) | | ||
| | `make remoteintegration` | Integration tests (remote) | | ||
| | `make localmachine` | Machine tests | | ||
| | `make localsystem` | System tests (BATS, local) | | ||
| | `make remotesystem` | System tests (BATS, remote) | | ||
| | `make test` | Run everything | | ||
|
|
||
| ## Integration Tests (Ginkgo, `test/e2e/`) |
There was a problem hiding this comment.
I think this all should just go into test/README.md instead of adding yet another file with partial info
| @echo "" | ||
| @echo "Examples:" | ||
| @echo " make localintegration FOCUS=\"podman run\" " | ||
| @echo " Run integration tests matching \"podman run\" description" | ||
| @echo " make localintegration FOCUS_FILE=run_test.go" | ||
| @echo " Run integration tests from a specific file" | ||
| @echo " make localmachine FOCUS_FILE=basic_test.go" | ||
| @echo " Run machine tests from a specific file" | ||
| @echo " make localintegration FOCUS=\"podman run\" GINKGO_PARALLEL=n" | ||
| @echo " Run a specific test without parallelism (useful for debugging)" |
There was a problem hiding this comment.
I am not sure this is particular helpful to show all the time.
It is important to remember that these targets only run on linux, would it not be better to have an example on how to build podman? And for the testing just tell users to look into test/README.md
|  | ||
|
|
||
| > **WARNING: Tests are destructive to your local environment.** | ||
| > Integration and system tests wipe `~/.local/share/containers` and `~/.config/containers` before running. This removes all local containers, images, pods, volumes, and machine instances. Run tests in a dedicated environment or VM, not on a workstation with data you care about. |
There was a problem hiding this comment.
integration tests should not wipe anything as they use custom dirs, only system tests do.
Though I guess the run in VM advice is solid regardless
There was a problem hiding this comment.
oh maybe out of scope for now but regarding the VM we likely should document the new CI behavior
hack/ci/ci.sh sys remote rootless fedora-current will run tests in the same VM as CI so is perfect for reproducing issues. All the user needs is to have lima installed. I did not check if the script works on macos though I believe it should be possible.
There was a problem hiding this comment.
Oh, this is good to know. I will create an issue for that.
| | `make localmachine` | Machine tests (`pkg/machine/e2e/`) | | ||
| | `make localsystem` | System tests (`test/system/`) | | ||
| | `make remotesystem` | System tests, remote client (`test/system/`) | | ||
| | `make test` | Run everything | |
There was a problem hiding this comment.
Did anybody ever use the run all target? That will take forever...
Overall I guess maybe as a note for new users I think for local testing we should strongly recommend the specific test path as the test suites take way to long otherwise.
|
The false positive linter errors: |
|
@Honny1 FYI if you link logs make sure to send the GH link not the resolved CDN link, the CDN links are timestamped and get invalidated soon. i.e. use links such as https://github.com/podman-container-tools/podman/actions/runs/27631989406/job/81709142760 |
|
LGTM for reference |
| | Command | What it does | | ||
| |---------|-------------| | ||
| | `make localunit` | Unit tests with coverage | | ||
| | `make localintegration` | Integration tests (`test/e2e/`) | |
There was a problem hiding this comment.
non-blocking nit: there's an expanded section on some of these later in this doc, how about:
| | `make localintegration` | Integration tests (`test/e2e/`) | | |
| | `make localintegration` | [Integration tests (`test/e2e/`)](#integration-tests) | |
Add ## descriptions to commonly used targets so they appear in make help. Replace the `wc -L` column-width approach (unavailable on macOS without coreutils) with awk-based alignment. Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Add a warning that tests wipe local container state, a quick-reference table of make targets, and examples for running specific integration, machine, system, and unit tests. Signed-off-by: Jan Rodák <hony.com@seznam.cz>
|
PTAL @podman-container-tools/podman-maintainers @podman-container-tools/podman-reviewers |
|
LGTM |
|
Self-merging since we have two LGTMs. |
danishprakash
left a comment
danishprakash
left a comment
There was a problem hiding this comment.
Sorry about the delay, the changes look good!
6 participants
docs/TESTING.mdas a concise, quick reference for running all test typesChecklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?