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

Remove special handling of stdlibs in test_deps_compat #215

Merged
merged 3 commits into from
Oct 24, 2023

Conversation

lgoettgens
Copy link
Collaborator

@lgoettgens lgoettgens commented Oct 17, 2023

Resolves #212.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #215 (b662e48) into master (69a7655) will decrease coverage by 0.93%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

❗ Current head b662e48 differs from pull request most recent head 40176e2. Consider uploading reports for the commit 40176e2 to get more accurate results

@@            Coverage Diff             @@
##           master     #215      +/-   ##
==========================================
- Coverage   89.63%   88.71%   -0.93%     
==========================================
  Files          10       10              
  Lines         415      505      +90     
==========================================
+ Hits          372      448      +76     
- Misses         43       57      +14     
Flag Coverage Δ
unittests 88.71% <ø> (-0.93%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/deps_compat.jl 70.27% <ø> (-8.31%) ⬇️
src/utils.jl 89.85% <ø> (+4.60%) ⬆️

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lgoettgens lgoettgens merged commit 44c89fd into master Oct 24, 2023
18 checks passed
@lgoettgens lgoettgens deleted the lg/stdlib-compat branch October 24, 2023 14:37
@vtjnash
Copy link
Contributor

vtjnash commented Nov 22, 2023

This (or something surrounding it) seems to have caused quite a few failures in PkgEval, which reduces the overall testing coverage in the ecosystem. Is there something we can do about this? For example:
https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/1fc9d54_vs_72cd63c/Quadmath.primary.log
https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/1fc9d54_vs_72cd63c/AbstractTrees.primary.log
https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/1fc9d54_vs_72cd63c/ManualMemory.primary.log

Looking at the apparent red cliff in the report: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/1fc9d54_vs_72cd63c/report.html

Some don't seem precisely related, as the error appears to complain about packages (e.g. BenchmarkTools) that don't appear to even be installed in the log?

@lgoettgens
Copy link
Collaborator Author

Yes, parts of the failures you linked are due to this change. But a lot are due to checking extras and weakdeps in general since #202. So every package declaring Aqua as a extra-Dep without a compat bound will fail as well.

The clean solution would be to change all failing packages to either fix the issues there or (easier) add a compat entry for Aqua 0.7 there.
If you are interested in PkgEval in particular, I can think about changing some kwargs in the default test list depending on the PkgEval ENV variable, so by default PkgEval testing would only use a subset of Aqua tests. Would that be something you are interested in? If the list of failing packages is rather short, I think this is not worth the effort and we should instead provide PRs to them. I unfortunately don't really feel confident to condense such a list from the PkgEval report you linked above.

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

Successfully merging this pull request may close these issues.

Adapt to change in compat requirements in the general registry
3 participants