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

CI should require presence of test/runtest.jl for new packages #313

Open
oxinabox opened this issue Dec 28, 2020 · 4 comments
Open

CI should require presence of test/runtest.jl for new packages #313

oxinabox opened this issue Dec 28, 2020 · 4 comments

Comments

@oxinabox
Copy link
Contributor

oxinabox commented Dec 28, 2020

A much weaker version of #120
and similar to #261 (though less crucial)

Even if we are not going to run it, or check that it contains something meaningful,
I think it is worth checking the bare minimum effort has been put into making sure the package is good.
At least be aware enough of what is going on to know that packages have these.

We used to have this requirement at one point for registring packages back in 0.4 days.

@DilumAluthge
Copy link
Member

Makes sense to me

@KristofferC
Copy link
Member

It seems unnecessary to add friction to registering a package that can just be circumvented with a @test 1==1 (which is what people that didn't add any tests in the first place likely will do).

@ericphanson
Copy link
Member

The friction is only in the case where there are no tests, so if we want to encourage tests, that seems like a good place to add friction, no?

Sure, it can be easily circumvented, but it’s at least a nudge in the right direction. I think it would also help set the expectation that tests are needed, eg a new developer might not know they’re something they should have, and learn about it from an automerge failure.

In general I think it’s fine for automerge to be opinionated in order to try to help nudge the package ecosystem in a good direction (since RegistryCI’s biggest client, the General registry, is a shared resource we want to keep healthy, not just an easy way to access random pieces of code).

@ericphanson
Copy link
Member

With the technology of ~~~tokei~~~ (https://github.com/XAMPPRocky/tokei, packaged as Tokei_jll), we could count lines of Julia code in /test (ref #351 for /src). That could be used to create a more stringent requirement than just that tests/runtests.jl exists (but still pretty cirumventable, just write @test true a bunch of times).

My opinion is that we probably should not do this (at least at this point), and just start with checking that test/runtests.jl exists. IMO that covers most of the bases regarding getting new devs up to speed regarding the package expectations and where to put tests, without being too burdensome.

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

No branches or pull requests

4 participants