docs: add local unit test and linting instructions to CONTRIBUTING.md - #3035
docs: add local unit test and linting instructions to CONTRIBUTING.md#3035ttuffin wants to merge 2 commits into
Conversation
The existing docs only covered integration tests. Unit tests require tox rather than raw pytest or ansible-test due to the ansible_collections namespace resolution needed for fully-qualified imports. Linting requires black-lint and flake8-lint tox environments. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Hi @alinabuzachis @GomathiselviS, I do not have permission to add reviewers. Can you please take a look at this? |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 57s |
tremble
left a comment
tremble
left a comment
There was a problem hiding this comment.
Thanks for taking the time to submit this PR.
There are actually more than just the two linters in play. This was already documented, however it's not necessarily where folks are looking for it (as evidenced by you not finding it):
| To run all unit tests: | ||
|
|
||
| ```bash | ||
| tox -e ansible2.20-py312-without_constraints |
There was a problem hiding this comment.
alternatively
tox -m unit-oldest or tox -m unit-newest
|
@tremble thanks for the review. Based on your comment about this already being covered in dev_guidances, do you still see value in adding this information to the contributing guide? Or shall I close the PR? |
mjohns91
left a comment
mjohns91
left a comment
There was a problem hiding this comment.
I don't think adding the details here for local testing is a problem. Seems reasonable enough.
Co-authored-by: Mark Chappell <mchappel@redhat.com>
Docs Build 📝Thank you for contribution!✨ The docsite for this PR is available for download as an artifact from this run: You can compare to the docs for the File changes:
Click to see the diff comparison.NOTE: only file modifications are shown here. New and deleted files are excluded. diff --git a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/s3_object_info_module.html b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/s3_object_info_module.html
index b16d20b..f8d9ad4 100644
--- a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/s3_object_info_module.html
+++ b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/s3_object_info_module.html
@@ -523,7 +523,7 @@ see <a class="reference internal" href="#ansible-collections-amazon-aws-s3-objec
<span class="p p-Indicator">-</span><span class="w"> </span><span class="nt">name</span><span class="p">:</span><span class="w"> </span><span class="l l-Scalar l-Scalar-Plain">Retrieve keys that begin with the prefix /my/desired/</span>
<span class="w"> </span><span class="nt">amazon.aws.s3_object_info</span><span class="p">:</span>
-<span class="w"> </span><span class="nt">bucket</span><span class="p">:</span><span class="w"> </span><span class="l l-Scalar l-Scalar-Plain">mybucket</span>
+<span class="w"> </span><span class="nt">bucket_name</span><span class="p">:</span><span class="w"> </span><span class="l l-Scalar l-Scalar-Plain">mybucket</span>
<span class="w"> </span><span class="nt">prefix</span><span class="p">:</span><span class="w"> </span><span class="l l-Scalar l-Scalar-Plain">/my/desired/</span>
</pre></div>
</div>
|
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 23s |
|
|
||
| ### Linting | ||
|
|
||
| Two linters are enforced in CI. Run them against changed files before submitting a PR: |
There was a problem hiding this comment.
I believe that there are other linters that run in CI. Looking at:
amazon.aws/.github/workflows/linters.yml
Line 11 in 14294b0
That should run all the linters with labels = lint in the tox config such as
Line 129 in 14294b0
So this section could be reduced down to tox -m lint instead of individual commands.
| tox -e flake8-lint -- <path/to/file> | ||
| ``` | ||
|
|
||
| To auto-fix formatting issues (rather than just checking), use the `black` environment: |
There was a problem hiding this comment.
| To auto-fix formatting issues (rather than just checking), use the `black` environment: | |
| To automatically fix any formatting issues, use the following command: |
|
|
||
| ```bash | ||
| tox -e ansible2.20-py312-without_constraints | ||
| ``` |
There was a problem hiding this comment.
I feel like this is somewhat misleading. I ran into this myself recently while looking at a pull request in this collection. There are multiple combinations of ansible-core and Python versions in the envlist and CI runs the lot.
Reading "all unit tests" might cause contributors to think they've fully covered their changes and not just one combo. So I'd rephrase to say something like:
"To run a specific environment that matches your Python version:" or something like that.
For all unit tests, I believe you would need tox -m unit.
| ``` | ||
|
|
||
| See `tox.ini` for the full list of available environments (ansible-core versions and | ||
| Python version combinations). |
There was a problem hiding this comment.
What about recommending tox list to discover available environments? That might be more straightforward for folks than digging through tox.ini.
| > **Note:** Do not use `pytest` or `ansible-test units` directly — they will fail with | ||
| > `ModuleNotFoundError: No module named 'ansible_collections'` because the collection | ||
| > namespace is not on `sys.path` without the tox setup. | ||
|
|
There was a problem hiding this comment.
I feel like tox -e diff-cover is worth a mention in here.
Line 89 in 14294b0
4 participants
I realised there was a lack of clear documentation for running unit tests and linters while developing the fix for #3034. The existing docs only covered integration tests. Unit tests require tox rather than raw pytest or ansible-test due to the ansible_collections namespace resolution needed for fully-qualified imports. Linting requires black-lint and flake8-lint tox environments.
ISSUE TYPE