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.

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

[MTSRE-298] Refactor integration test suite #99

Merged
merged 7 commits into from
Nov 22, 2021
Merged

[MTSRE-298] Refactor integration test suite #99

merged 7 commits into from
Nov 22, 2021

Conversation

mayankshah1607
Copy link
Contributor

@mayankshah1607 mayankshah1607 commented Nov 16, 2021

This PR introduces the testify/suite library for integration tests. This mechanism allows us to group theses tests into a common integrationTestSuite and add proper setup and teardown for it.

  • Use SetupSuite and TeardownSuite methods for the test suite
  • De-duplicate test code and combine catalog_source_test.go, namespaces_test.go, and subscription_test.go into a single test with each of them as a subtest in addon_test.go
  • Minor refactors within the integration/ package to adapt to the suite library
  • Move Addon object declarations into re-usable fixtures in fixtures_test.go, thereby removing redundant Addon object initializations

Removal of parallel test execution

This PR also removes parallel execution of tests becausetestify/suite has poor support for parallel test execution - stretchr/testify#187 . Running tests within a test suite in parallel causes the following problems:

  • Raciness - test suite is torn down while some tests are still running
  • Pod logs printing happens out-of-order

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 16, 2021

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 16, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 16, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mayankshah1607

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 16, 2021
@mayankshah1607 mayankshah1607 marked this pull request as ready for review November 17, 2021 05:38
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 17, 2021
@mayankshah1607 mayankshah1607 changed the title Refactor integration tests [MTSRE-298] Refactor integration test suite Nov 17, 2021
@mayankshah1607
Copy link
Contributor Author

/retest

1 similar comment
@mayankshah1607
Copy link
Contributor Author

/retest

@thetechnick
Copy link
Contributor

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2021
@mayankshah1607 mayankshah1607 changed the title [MTSRE-298] Refactor integration test suite WIP - [MTSRE-298] Refactor integration test suite Nov 18, 2021
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 18, 2021
@mayankshah1607 mayankshah1607 changed the title WIP - [MTSRE-298] Refactor integration test suite [MTSRE-298] Refactor integration test suite Nov 18, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 18, 2021
Signed-off-by: Mayank Shah <[email protected]>
@mayankshah1607
Copy link
Contributor Author

/retest

2 similar comments
@mayankshah1607
Copy link
Contributor Author

/retest

@mayankshah1607
Copy link
Contributor Author

/retest

@mayankshah1607
Copy link
Contributor Author

/retest

1 similar comment
@mayankshah1607
Copy link
Contributor Author

/retest

@mayankshah1607
Copy link
Contributor Author

/hold

@thetechnick
Copy link
Contributor

/lgtm

Awesome work!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2021
@thetechnick
Copy link
Contributor

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2021
@openshift-merge-robot openshift-merge-robot merged commit ad40887 into openshift:main Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants