Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ec2_instance] refactor integration tests and other fixes #1404

Closed
wants to merge 3 commits into from
Closed

[ec2_instance] refactor integration tests and other fixes #1404

wants to merge 3 commits into from

Conversation

abikouo
Copy link
Contributor

@abikouo abikouo commented Mar 3, 2023 •
edited
Loading

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

ec2_instance
ec2_instance_info

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/1e32747c7ccd4f238cefc28d17f9bb54

✔️ ansible-galaxy-importer SUCCESS in 3m 53s
✔️ build-ansible-collection SUCCESS in 13m 01s
❌ ansible-test-sanity-aws-ansible-python38 FAILURE in 11m 49s (non-voting)
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 26s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 09s
✔️ ansible-test-sanity-aws-ansible-2.14 SUCCESS in 10m 16s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 8m 08s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 9m 05s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 9m 15s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 8m 18s
✔️ cloud-tox-py3 SUCCESS in 7m 23s
✔️ ansible-test-changelog SUCCESS in 4m 20s
✔️ ansible-test-splitter SUCCESS in 4m 45s
❌ integration-amazon.aws-1 FAILURE in 21m 36s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/295f9a7482024b95946b6f243dc15d52

✔️ ansible-galaxy-importer SUCCESS in 4m 02s
✔️ build-ansible-collection SUCCESS in 12m 26s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 8m 36s (non-voting)
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 03s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 12m 29s
✔️ ansible-test-sanity-aws-ansible-2.14 SUCCESS in 12m 06s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 9m 27s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 8m 10s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 9m 43s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 9m 45s
✔️ cloud-tox-py3 SUCCESS in 7m 10s
✔️ ansible-test-changelog SUCCESS in 4m 14s
✔️ ansible-test-splitter SUCCESS in 4m 53s
✔️ integration-amazon.aws-1 SUCCESS in 21m 44s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/4be7ff6e11d64e1e8431869a5597b452

✔️ ansible-galaxy-importer SUCCESS in 4m 03s
✔️ build-ansible-collection SUCCESS in 12m 22s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 00s (non-voting)
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 11s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 58s
✔️ ansible-test-sanity-aws-ansible-2.14 SUCCESS in 8m 31s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 11m 09s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 9m 16s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 11m 09s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 8m 03s
✔️ cloud-tox-py3 SUCCESS in 7m 25s
✔️ ansible-test-changelog SUCCESS in 4m 20s
✔️ ansible-test-splitter SUCCESS in 5m 05s
✔️ integration-amazon.aws-1 SUCCESS in 19m 38s
Skipped 43 jobs

@ansibullbot
Copy link

@abikouo This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage labels Mar 31, 2023
abikouo added 3 commits May 4, 2023 16:05
@abikouo
multiple changes on ec2_instances integration tests, fix check mode i…
53b423e 
…ssue with ec2_instance when attaching new network interface, add new parameter to ec2_instance_info to describe specific instance attribute
@abikouo
fix tests
cc7a7d2
@abikouo
darker
6d4b607
@abikouo abikouo force-pushed the ec2_instance_optim branch from d8c3fd2 to 6d4b607 Compare May 4, 2023 14:07
@abikouo abikouo requested review from jillr, tremble and gravesm May 4, 2023 14:09
@abikouo abikouo removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR community_review labels May 4, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/f6be3d96cd6f459bb3d950816a562786

✔️ ansible-galaxy-importer SUCCESS in 3m 38s
✔️ build-ansible-collection SUCCESS in 14m 19s
✔️ ansible-test-splitter SUCCESS in 5m 34s
✔️ integration-amazon.aws-1 SUCCESS in 20m 47s
Skipped 43 jobs

@jillr
Copy link
Collaborator

jillr commented Jun 1, 2023

@abikouo Can you please explain the changes in this PR and how they will address the run time? For example, what process or logic did you use to determine what test changes to make and what new behaviour or outcome does the refactor achieve. Is this the smallest changeset that can reasonably be created, or could we take an incremental approach?

From the PR title and commit messages it seems like there might be several code changes along with the test refactoring. Individual code changes should have their own PRs, separate from the test refactoring, to make each change easier to review.

@abikouo
Copy link
Contributor Author

abikouo commented Jun 2, 2023

@abikouo Can you please explain the changes in this PR and how they will address the run time? For example, what process or logic did you use to determine what test changes to make and what new behaviour or outcome does the refactor achieve. Is this the smallest changeset that can reasonably be created, or could we take an incremental approach?

From the PR title and commit messages it seems like there might be several code changes along with the test refactoring. Individual code changes should have their own PRs, separate from the test refactoring, to make each change easier to review.

Other changes not directly related to the EC2_instance integration tests have already been moved into separated pull requests #1577 and #1576
Regarding this one, I will add more details on the pull request description on how this is improving tests

@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 12, 2023
@tremble
Copy link
Contributor

tremble commented Nov 30, 2023

this PR's badly bit rotted, and I'm not sure dropping a ton of tests is the best idea. closing

@tremble tremble closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jillr jillr Awaiting requested review from jillr

@tremble tremble Awaiting requested review from tremble

@gravesm gravesm Awaiting requested review from gravesm

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ec2_instance - attach network interfaces issue when using check_mode
5 participants
@abikouo @ansibullbot @jillr @tremble @alinabuzachis