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

classify tests and extend coverage #745

Merged
merged 23 commits into from
Dec 7, 2021
Merged

classify tests and extend coverage #745

merged 23 commits into from
Dec 7, 2021

Conversation

clux
Copy link
Member

@clux clux commented Dec 6, 2021

This pr clarifies the distinction between our different types of tests and writes a policy on when to write a test for what. The key change for that is in the new CONTRIBUTING.md.

Classification: integration reclassified as e2e. New integration is cargo test --ignored (using a live Client)

New tests

Some tests imported and re-purposed from examples with new invariants:

  • integration tests use disjoint set of objects
  • integration tests now can run in parallel

This lets us improve the coverage without having to build the very expensive example setup with tarpaulin (and also lets us potentially run this class of tests more easily across clusters).

Coverage Improvements

Almost a 10% bump locally. After merge: codecov (NB: it doesn't count derive very well, so it's still misleading).

  • kube-client/src/lib.rs -> integration tests that need client and core (web sockets, pod creation, api usage, discovery)
  • kube/src/lib.rs -> integration tests that need a combination of runtime, derive, and client (crd usage, crd discovery)

Also some more sporadic integration tests in runtime's eventrecorder.

Test Policy

We need a test policy and proof that we uphold a test policy in general for #737 and this should be sufficient for the policy, and pointing at a constant line/upward trend on coverage should be enough to prove it is upheld.

@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2021

Codecov Report

Merging #745 (d6ed190) into master (3e92a67) will increase coverage by 9.85%.
The diff coverage is 94.51%.

❗ Current head d6ed190 differs from pull request most recent head e2c6201. Consider uploading reports for the commit e2c6201 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #745      +/-   ##
==========================================
+ Coverage   60.34%   70.19%   +9.85%     
==========================================
  Files          52       53       +1     
  Lines        3051     3352     +301     
==========================================
+ Hits         1841     2353     +512     
+ Misses       1210      999     -211     
Impacted Files Coverage Δ
kube-runtime/src/wait.rs 35.71% <0.00%> (+1.56%) ⬆️
kube-client/src/lib.rs 92.74% <92.74%> (ø)
kube-runtime/src/events.rs 92.45% <94.73%> (+92.45%) ⬆️
kube/src/lib.rs 97.72% <96.82%> (-2.28%) ⬇️
kube-core/src/object.rs 76.66% <100.00%> (+43.33%) ⬆️
kube-core/src/subresource.rs 75.72% <100.00%> (+16.78%) ⬆️
kube-core/src/util.rs 100.00% <100.00%> (+100.00%) ⬆️
kube-core/src/dynamic.rs 66.66% <0.00%> (-4.77%) ⬇️
kube-core/src/request.rs 95.00% <0.00%> (+0.38%) ⬆️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e92a67...e2c6201. Read the comment docs.

@clux clux mentioned this pull request Dec 7, 2021
33 tasks
this forces our integration folder to be moved to e2e as it is the
heaviest type of test.

but crucially it leaves a path for us to run our integration tests in a
matrix against various clusters.

Signed-off-by: clux <[email protected]>
@clux clux changed the title more coverage experiments classify tests and extend coverage Dec 7, 2021
@clux clux mentioned this pull request Dec 7, 2021
6 tasks
@clux clux marked this pull request as ready for review December 7, 2021 20:05
@clux clux requested a review from a team December 7, 2021 20:05
Signed-off-by: clux <[email protected]>
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
kube-client/src/lib.rs Outdated Show resolved Hide resolved
kube-client/src/lib.rs Outdated Show resolved Hide resolved
@kazk
Copy link
Member

kazk commented Dec 7, 2021

I'll remove the requirement for integration check, and add e2e.

EDIT: Done

image

@clux
Copy link
Member Author

clux commented Dec 7, 2021

Ok, perfect. I am ready to merge this if you are happy with it.

@clux clux merged commit 8b768fd into master Dec 7, 2021
@clux clux deleted the cov-test branch December 7, 2021 23:47
@clux clux added this to the 0.65.0 milestone Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants