Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Standardize of tests/requirements.yml #230

Closed
ssbarnea opened this issue May 18, 2023 · 15 comments
Closed

Standardize of tests/requirements.yml #230

ssbarnea opened this issue May 18, 2023 · 15 comments

Comments

@ssbarnea
Copy link
Member

ssbarnea commented May 18, 2023

Summary

Apparently we currently have 18/19 collections that have a tests/requirements.yml file that is not loaded by any tool and that is not compatible with our official requirements.yml schema.

Current unsupported format looks inspired from old requirements v1 format:

---
integration_tests_dependencies:
 - community.general
 - amazon.aws >= 3.0.0
unit_tests_dependencies: []

In order to improve the development experience we should aim to find an answer to the following questions:

  • Can we avoid using two different file formats and schemas for two files with identical names?
  • Could we extend the existing root requirements.yml to allow it to specify test requirements? Obviously that this means that ansible-galaxy or hub should just ignore the unknown content instead of choking when encountering it.
  • Can we maybe make use of tags to mark dependencies that are used only for testing?
  • Do we need more than unit and integration type of dependencies?
  • What about roles? Current format does not allow users to define any standalone roles as being required for testing only.

Now is probably the right time to make suggestions and get these right and hopefully future proof.

Additional Information

Possible impact on other tools:

  • ansible-galaxy command seems to not be able to allow adding any keys in addition to roles: and collections: on a requirements.yml file. That is big bad because it does not allow us to extend the file for incorporating test requirements via new keys. If we try to add special tags for test requirements, they will install with older versions of ansible-galaxy that do not know to skip these tags. Bit bad, I wonder if core team can help use with some ideas here.
  • ansible-hub upload (TBD)
  • ansible-builder v3 makes indirect use of official requirements.yml files
  • ansible-lint and molecule are easier to fix regardless what we decide
---
roles:
  - name: geerlingguy.mysql
    src: https://github.com/geerlingguy/ansible-role-mysql
    version: 4.2.0
    tags: [test, integration] # <-- ansible-galaxy ignores these

collections:
  - name: https://galaxy.ansible.com/download/community-molecule-0.1.0.tar.gz
    tags: [test] # <-- ansible-galaxy ignores these
@bcoca
Copy link

bcoca commented May 18, 2023

simplest solution is not to alter any formats, just move to sudirs:

test/units/requirements.yml
test/integration/requirements.yml

And use existing supported ansible-galaxy format

@ssbarnea
Copy link
Member Author

@felixfontein I think that the proposal above has very good selling points. We will only have to split the current 18 files into two other files and the problem would be sorted. All files will have the same file format and tools using them might decide to ignore some of them.

@felixfontein
Copy link
Contributor

A Galaxy dependency file per directory sounds like a good idea.

Is there a standard directory for molecule tests in collections BTW?

@ssbarnea
Copy link
Member Author

I do have a PR in progress for changing linter/compat and making it recognize invalid requirements.yml file with a message like:

tests/requirements.yml file is not a valid Ansible requirements file. Only 'roles' and 'collections' keys are allowed at root level. Recognized valid locations are:
requirements.yml
roles/requirements.yml
collections/requirements.yml
tests/requirements.yml
tests/integration/requirements.yml
tests/unit/requirements.yml

We will have to fix those community collections manually to remove the undocumented/unsupported "integration_tests_dependencies" bits, so people will not get confused. I will do PRs for some of them, lucky we don't have so many.

ssbarnea added a commit to ssbarnea/community.network that referenced this issue May 21, 2023
@felixfontein
Copy link
Contributor

I created some more PRs for all collections I have locally checked out.

@mariolenz
Copy link
Contributor

I've had a look at community.vmware and I've seen that there is also a test-requirements.txt in the root folder.

@bcoca @felixfontein @ssbarnea Is this a file we should aim to get rid of, too? There are quite some collections in ansible-collections that have this file.

@ssbarnea
Copy link
Member Author

ssbarnea commented May 21, 2023

I agree with @mariolenz but we probably need to do this in gradual steps. For example I want to add an entire documentation page into the linter about where to put requirements, so we can tell people to follow it.

Regarding requirements.txt -- these are very different than ansible ones (.yml), do not confuse them. For know the linter does nothing about them.

BTW, I am quite happy to see that a solution slowly emerges here.

@mariolenz
Copy link
Contributor

@ssbarnea FYI I ran into a problem when I removed test-requirements.txt.

I agree that we should do this in gradual steps. Actually, I don't even know if this file is a problem... just wanted to mention it so you could have a look.

@felixfontein
Copy link
Contributor

The standard places for Python test requirements are tests/unit/requirements.txt for unit tests, and tests/integration/requirements.txt for integration tests. ansible-test has been using these for a long time (at least the former, I've never used the latter so far). test-requirements.txt is something I've never (actively) seen before. That's something that comes from the Ansible Zuul config: https://github.com/ansible/zuul-config/blob/master/roles/ansible-test/defaults/main.yaml#L28

No idea why it was introduced or by whom.

patchback bot pushed a commit to ansible-collections/community.aws that referenced this issue May 22, 2023
Switch to Ansible Galaxy compatible requirements files for tests

SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Mark Chappell
(cherry picked from commit e80bf93)
patchback bot pushed a commit to ansible-collections/community.aws that referenced this issue May 22, 2023
Switch to Ansible Galaxy compatible requirements files for tests

SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Mark Chappell
(cherry picked from commit e80bf93)
softwarefactory-project-zuul bot pushed a commit to ansible-collections/amazon.aws that referenced this issue May 22, 2023
) (#1565)

[PR #1564/6d62d39d backport][stable-5] Switch to Ansible Galaxy compatible requirements files for tests

This is a backport of PR #1564 as merged into main (6d62d39).
SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Mark Chappell
softwarefactory-project-zuul bot pushed a commit to ansible-collections/amazon.aws that referenced this issue May 22, 2023
) (#1566)

[PR #1564/6d62d39d backport][stable-6] Switch to Ansible Galaxy compatible requirements files for tests

This is a backport of PR #1564 as merged into main (6d62d39).
SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Mark Chappell
softwarefactory-project-zuul bot pushed a commit to ansible-collections/community.aws that referenced this issue May 22, 2023
…tible requirements files for tests (#1817)

[PR #1816/e80bf933 backport][stable-5] Switch to Ansible Galaxy compatible requirements files for tests

This is a backport of PR #1816 as merged into main (e80bf93).
SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Mark Chappell
@mariolenz
Copy link
Contributor

test-requirements.txt is something I've never (actively) seen before. That's something that comes from the Ansible Zuul config: https://github.com/ansible/zuul-config/blob/master/roles/ansible-test/defaults/main.yaml#L28

I've tried to remove the file but then Zuul fails with:

ERROR: Could not open requirements file: [Errno 2] No such file or directory: '/home/zuul-worker/.ansible/collections/ansible_collections/community/vmware/test-requirements.txt'

grafik

softwarefactory-project-zuul bot pushed a commit to ansible-collections/community.vmware that referenced this issue May 22, 2023
Andersson007 pushed a commit to ansible-collections/community.network that referenced this issue May 23, 2023
patchback bot pushed a commit to ansible-collections/community.network that referenced this issue May 23, 2023
patchback bot pushed a commit to ansible-collections/community.network that referenced this issue May 23, 2023
Andersson007 pushed a commit to ansible-collections/community.network that referenced this issue May 23, 2023
Related: ansible-community/community-topics#230
(cherry picked from commit 202da7c)

Co-authored-by: Sorin Sbarnea <sorin.sbarnea@gmail.com>
Andersson007 pushed a commit to ansible-collections/community.network that referenced this issue May 23, 2023
Related: ansible-community/community-topics#230
(cherry picked from commit 202da7c)

Co-authored-by: Sorin Sbarnea <sorin.sbarnea@gmail.com>
softwarefactory-project-zuul bot pushed a commit to ansible-collections/community.aws that referenced this issue May 23, 2023
…) (#1818)

[PR #1816/e80bf933 backport][stable-6] Switch to Ansible Galaxy compatible requirements files for tests

This is a backport of PR #1816 as merged into main (e80bf93).
SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Mark Chappell
alinabuzachis added a commit to alinabuzachis/amazon.cloud that referenced this issue May 26, 2023
…irements.yml file

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
alinabuzachis added a commit to alinabuzachis/amazon.cloud that referenced this issue May 26, 2023
…irements.yml file

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
alinabuzachis added a commit to alinabuzachis/amazon.cloud that referenced this issue May 26, 2023
…irements.yml file

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
alinabuzachis added a commit to alinabuzachis/cloud.terraform that referenced this issue May 26, 2023
…irements.yml file

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
alinabuzachis added a commit to ansible-collections/cloud.terraform that referenced this issue May 26, 2023
…irements.yml file (#81)

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
softwarefactory-project-zuul bot added a commit to ansible-collections/ansible.posix that referenced this issue May 31, 2023
Switch to Ansible Galaxy compatible requirements files for tests

SUMMARY
See ansible-community/community-topics#230.
ISSUE TYPE

Test Pull Request

COMPONENT NAME
test requirements files

Reviewed-by: Sorin Sbarnea <sorin.sbarnea@gmail.com>
@mariolenz
Copy link
Contributor

@ssbarnea Please close this issue if done, or open a new forum topic and then close the issue with a pointer to the new discussion: Community-topics: Archiving the repo

@felixfontein
Copy link
Contributor

I think this is kind of standardized, so we can close it. The only thing left is to document the result somewhere. Because right now, it is nowhere documented as a standard...

@ssbarnea is this somewhat documented in the ansible-lint docs?

@Andersson007
Copy link
Contributor

as it's still possible to write comments here, let's close it and move this to the forum if needed, thanks everyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants