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

test: Apply deployments in BeforeAll rather than BeforeEach #11514

Merged
merged 1 commit into from May 13, 2020
Merged

test: Apply deployments in BeforeAll rather than BeforeEach #11514

merged 1 commit into from May 13, 2020

Conversation

jrajahalme
Copy link
Member

Reduce test churn by running multiple tests on the same set of
resources. This seems to speed up the test by couple of minutes (10
vs. 12 minutes).

Signed-off-by: Jarno Rajahalme jarno@covalent.io

@jrajahalme
test: Apply deployments in BeforeAll rather than BeforeEach
d32efae 
Reduce test churn by running multiple tests on the same set of
resources. This seems to speed up the test by couple of minutes (10
vs. 12 minutes).

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme jrajahalme added area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. labels May 13, 2020
@jrajahalme jrajahalme requested a review from a team as a code owner May 13, 2020 04:17
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 13, 2020
@jrajahalme
Copy link
Member Author

test-me-please

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 37.846% when pulling d32efae on pr/jrajahalme/test-apply-deployments-in-beforeall into d147e15 on master.

nebril
nebril reviewed May 13, 2020
View reviewed changes
Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I would like to get second opinion from test authors.

nebril
nebril approved these changes May 13, 2020
View reviewed changes
Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like affected tests don't change any deployment state, let's rerun tests and get this merged, thanks!

@nebril
Copy link
Member

nebril commented May 13, 2020

retest-runtime

@nebril
Copy link
Member

nebril commented May 13, 2020

retest-net-next

@nebril
Copy link
Member

nebril commented May 13, 2020

retest-gke

@jrajahalme
Copy link
Member Author

@nebril net-next test failed to a timeout, and the logs look like the test has been making progress for the whole 3 hours without any long gaps. Maybe this job needs to be chopped up if it routineously takes 3+ hours?

@jrajahalme
Copy link
Member Author

retest-net-next

@jrajahalme
Copy link
Member Author

All GKE failures are from unrelated tests, retesting.

@jrajahalme
Copy link
Member Author

retest-gke

@aanm aanm merged commit a7ec82e into master May 13, 2020
1.8.0 automation moved this from In progress to Merged May 13, 2020
@aanm aanm deleted the pr/jrajahalme/test-apply-deployments-in-beforeall branch May 13, 2020 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nebril nebril nebril approved these changes

Assignees
No one assigned
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jrajahalme @coveralls @nebril @aanm