Skip to content

Make Test a weak dependency#521

Open
devmotion wants to merge 7 commits into
masterfrom
dw/testutils
Open

Make Test a weak dependency#521
devmotion wants to merge 7 commits into
masterfrom
dw/testutils

Conversation

@devmotion

Copy link
Copy Markdown
Member

I'm not sure if this is a good idea at all and if it works as intended - BUT I've encountered multiple repos in the last few months where people did not want to add dependencies on Test. So I was wondering if we could get away with making the test utilities an extension of the package and wanted to try it in KernelFunctions.

IMO, the main problems are that 1) the code is a bit more complex and contains a "workaround" (since only overloaded but not new functions in extensions are accessible for the user) and 2) the extension is always loaded when testing the PR in the REPL.

I assume the reason for 2) might be that the Test stdlib is included in the default system image (?). Or are stdlibs not supported as weakly dependencies (I guess @KristofferC should know this?)?

Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread ext/KernelFunctionsTestExt.jl Outdated
Comment thread src/TestUtils.jl Outdated
@codecov

codecov Bot commented Jun 26, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 2.63158% with 74 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.51%. Comparing base (095b8d0) to head (ff5a142).
⚠️ Report is 58 commits behind head on master.

Files with missing lines Patch % Lines
ext/KernelFunctionsTestExt.jl 0.00% 74 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (095b8d0) and HEAD (ff5a142). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (095b8d0) HEAD (ff5a142)
4 2
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #521       +/-   ##
===========================================
- Coverage   67.34%   31.51%   -35.83%     
===========================================
  Files          52       53        +1     
  Lines        1384     1377        -7     
===========================================
- Hits          932      434      -498     
- Misses        452      943      +491     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

devmotion and others added 2 commits June 27, 2023 01:53
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@devmotion

Copy link
Copy Markdown
Member Author

This approach was adopted by eg AbstractFFTs, TranscodingStreams, and (soon) Distributions. I think we should make Test a weak dependency to reduce the number of dependencies and to be nice to other packages that made Test a weak dependency.

Comment thread src/TestUtils.jl Outdated
Comment thread src/TestUtils.jl Outdated
devmotion and others added 2 commits October 20, 2023 10:40
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@willtebbutt

Copy link
Copy Markdown
Member

I agree that we should do this -- I'm contemplating doing it with some of my other repos as well anyway.

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.

2 participants