-
Notifications
You must be signed in to change notification settings - Fork 342
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
Document how to run integration tests more clearly #924
Comments
…#794) Add abort multipart upload and expire obj del markers to s3 lifecycle Depends-On: ansible/ansible-zuul-jobs#1247 SUMMARY Fixes #365 #796 ISSUE TYPE Feature Pull Request COMPONENT NAME s3_lifecycle ADDITIONAL INFORMATION I have not run integration tests yet because of #793. I'm unsure about how to name and structure the new arguments. Do I nest them to match the API, or flatten them to match existing arguments? Reviewed-by: Alina Buzachis <None> Reviewed-by: Matthew Davis <None> Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> Reviewed-by: Markus Bergholz <git@osuv.de>
…#794) Add abort multipart upload and expire obj del markers to s3 lifecycle Depends-On: ansible/ansible-zuul-jobs#1247 SUMMARY Fixes #365 #796 ISSUE TYPE Feature Pull Request COMPONENT NAME s3_lifecycle ADDITIONAL INFORMATION I have not run integration tests yet because of #793. I'm unsure about how to name and structure the new arguments. Do I nest them to match the API, or flatten them to match existing arguments? Reviewed-by: Alina Buzachis <None> Reviewed-by: Matthew Davis <None> Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> Reviewed-by: Markus Bergholz <git@osuv.de> (cherry picked from commit ed3a7a0)
…#794) (#864) [PR #794/ed3a7a0b backport][stable-2] Add abort multipart upload and expire obj del markers to s3 lifecycle This is a backport of PR #794 as merged into main (ed3a7a0). Depends-On: ansible/ansible-zuul-jobs#1247 SUMMARY Fixes #365 #796 ISSUE TYPE Feature Pull Request COMPONENT NAME s3_lifecycle ADDITIONAL INFORMATION I have not run integration tests yet because of #793. I'm unsure about how to name and structure the new arguments. Do I nest them to match the API, or flatten them to match existing arguments?
Agree, things are a bit confusing. It takes some time to understand how pieces fit together. WARNING: Excluding tests marked "cloud/aws" which require config
(see "/home/dev/ansible/ansible/test/lib/ansible_test/config/cloud-config-aws.ini.template"): ec2_group I have found this link that mentions:
So you don't need to store the configuration file your virtual environment, but in the λ git issue-628* → pwd
/home/dev/ansible_collections/amazon/aws/tests/integration
λ fedora-dev integration → λ git issue-628* → ls -al
total 12
drwxr-xr-x. 1 dev dev 116 Apr 18 19:03 .
drwxr-xr-x. 1 dev dev 126 Apr 18 19:03 ..
-rw-r--r--. 1 dev dev 894 Apr 18 19:00 cloud-config-aws.ini
... |
Damn, |
I've bumped this issue over to amazon.aws which is where the more comprehensive documentation for amazon.aws and community.aws lives. |
Hi folks - we have some generic 'contributing to collections' documentation at https://docs.ansible.com/ansible/latest/community/create_pr_quick_start.html And some about testing at: If we link to these from the CONTRIBUTOR file, does this cover a lot of what's been pointed out here as problematic? (things that aren't specific to AWS collection development)... |
Ok I'm trying to write another PR, and running into the same issues. @samccann - Yes that's helpful. I started by going to the README at the top of this repo, and looking at the contribution section. I did not even notice the CONTRIBUTING.md file. Instead I followed where the README pointed and found this, which doesn't work because it refers to a file that no longer exists. In your doc I'm not sure which directory I think an important change would be to modify this warning from
To say:
And the other change to make things clearer is to update this page to not mention |
Another issue I had is that I can't run |
Another issue I just found: Integration tests fail if the account is not empty. The lambda integration tests assume there are no other lambdas in the account. I just swapped my region to one I don't use. But this is just another thing that should be documented in the README or CONTRIBUTING files in this repo. |
After running the integration tests, there is a new file I'll add a |
This generally shouldn't be the case, our tests sometimes get run in parallel. Was the error permissions related? |
No. The error was for integration testing lambda. It was an assertion error.
If the freshly created lambda for this test is not the first lambda in the list (because a pre-existing lambda is), then this assertion fails. Perhaps it's just that this specific test needs to be made a bit smarter. |
Yup, that's a bug in that test. Such behaviour is "bad", and will result in our integration tests being flaky when run in CI |
…ts (#1276) add .gitignore for inventory automatically created by integration tests SUMMARY Helps fix #924 ISSUE TYPE Bugfix Pull Request COMPONENT NAME tests ADDITIONAL INFORMATION When running integration tests, a file is generated at tests/integration/inventory. This file contains paths specific to the test machine. I assume this shouldn't be committed. So I've added a .gitignore for it. The leading slash is to make it not apply recursively. (Stack Overflow) Reviewed-by: Mark Chappell <None>
docs: Add more information about running integration tests SUMMARY Adds content to CONTRIBUTING.md, and rearranges some red-herring links. Fixes: #924 ISSUE TYPE Docs Pull Request COMPONENT NAME CONTRIBUTING.md ADDITIONAL INFORMATION Reviewed-by: Mark Chappell <None> Reviewed-by: Sandra McCann <samccann@redhat.com> Reviewed-by: Matthew Davis <None> Reviewed-by: Alina Buzachis <None>
Update contributing to sync with amazon.aws Fixes: #1607 SUMMARY Update CONTRIBUTING.md to sync with amazon.aws and pick up various recent improvements (See also ansible-collections/amazon.aws#924) ISSUE TYPE Docs Pull Request COMPONENT NAME CONTRIBUTING.md ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis
Update contributing to sync with amazon.aws Fixes: ansible-collections#1607 SUMMARY Update CONTRIBUTING.md to sync with amazon.aws and pick up various recent improvements (See also ansible-collections/amazon.aws#924) ISSUE TYPE Docs Pull Request COMPONENT NAME CONTRIBUTING.md ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis
Summary
I'm trying to write a PR, and run integration tests locally. It's incredibly confusing. The documentation is spread across dozens of pages. I'm really struggling to figure out how to run integration tests, and how to ensure they're run against my clone not the stable release that's already installed. Can you please add something straightforward to the README of this repo?
There are specific points where I get stuck. And I've suggested a new section to put in the README to fix this issue.
Issue Type
Documentation Report
Component Name
docs
Ansible Version
Collection Versions
Configuration
$ ansible-config dump --only-changed
(empty)
OS / Environment
No response
Additional Information
The problem
For context, I have used this collection, but I've never installed or managed collections directly. I don't think it should be assumed that people wanting to submit PRs to this repo know the semantics of things like
COLLECTIONS_PATH
.The top-level README for this repo doesn't say how to run integrations, or where or how to install this collection (from a git checkout). It only explains how to install the stable release, and links to pages saying how to write integration tests.
The contributing file does not say how to run integration tests. It just points to the integration test page of the module development guide. Step 1 of that guide refers to file
hacking/env-setup
. There is no such file in this repo!I found another file saying that I can skip this step if
ansible-test
is installed. Since collections were split off from Ansible core,ansible-test
is going to always be installed. So I'm not sure why that's mentioned at all. Can we delete that part? Or link to a different page?I don't think the docs for this collection link to anything telling me which folder to run
ansible-test
in.Pages like this collection testing guide tell me how to write collections, and they tell me what command to use. But they don't tell me which directory to run in, or how to ensure I'm testing my checkout, not the stable release that is installed on the machine already.
When I try to run
ansible-test
at the top of this repo, I get:Note that the only folder on my machine called
ansible_collections
already has the latest stable release of this collection installed.I eventually found yet another documentation page telling me to clone into. But the documentation is so dispersed that I'm unable to find it again now while typing this up.
The answer was
~/ansible/collections/ansible_collections/community/aws
, and then you put a different directory to that inCOLLECTIONS_PATHS
. Do Ansible developers really spend all their time nested a hundred folders deep to do work?Then when I run the unit tests, I get:
Hmm, that's odd. I'm running this on EC2, so
boto
should be able to find IAM credentials. Ok, well I open that file and do what it says. I copy/home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini.template
to/home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini
. But what do I put forsecurity_token
if I have no security token? Do I leave it assecurity_token: @SECURITY_TOKEN
or have it empty, or''
, ornull
, or delete the line? The file doesn't say.After doing exactly what the file told me to, I re-run and get the same error. So now I'm stuck.
What do I do after I've edited cloud-config-aws.ini if I still get the same error?
Solution
Add a new section to the README, which says:
community.aws
toaws
, and save that into a directory heirachy ending inansible/collections/ansible_collections/community/
.COLLECTIONS_PATHS
environment variable thecollections/
directory which is 3 levels above your clone, with a trailing slash.For example:
This last command is temporary, only for this terminal session. If you want to do this permanently (which will override the stable release of this repo with your checkout), open
~/.bashrc
and addNow navigate to the top of the repo (e.g.
cd ~/dev/ansible/collections/ansible_collections/community/aws
).Run
This will list all the possible integration tests you can run.
To run one (e.g.
x
), use:You will get this error:
Open up that file and read the instructions there.
You need to copy and rename that file, removing the
.template
but leaving it in the same directory.Now edit that new file, to add the relevant IAM credentials.
If you do not need a session token, then TODO: someone tell me what to do with the session token line
Now re-run the previous
ansible-test
command.If you still get the same error, then you need to TODO: someone tell me what to do here.
Code of Conduct
The text was updated successfully, but these errors were encountered: