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

Don't ship the tests with iris #3416

Closed
bjlittle opened this issue Sep 24, 2019 · 9 comments
Closed

Don't ship the tests with iris #3416

bjlittle opened this issue Sep 24, 2019 · 9 comments

Comments

@bjlittle
Copy link
Member

Lift and shift the iris.tests out and up to the root directory (same level as setup.py) to avoid installing the tests with iris.

Also collapse the lib directory, it's not necessary.

@ajdawson
Copy link
Member

Removing the tests will mean it is not possible to run the tests on an installed version. That may or may not be OK, I'm not sure. It is worth considering this use case at least.

@bjlittle
Copy link
Member Author

Hey @ajdawson! Great to hear from you 😄

I definitely think it's worthwhile considering. Just though I'd have the debate here...

My line of thought was that if iris has been installed as a package then it's most likely a pucker packaged release, and it should just work. In that sense, I really wouldn't expect someone to run all the tests, as they'd also need to set-up the test data etc. I'm more inclined to think that a developer would run the tests, and they wouldn't do that from an installation, rather they'd pip install -e . (or similar) a development (or master) branch into an installation that they own i.e., miniconda - in which case they'd have access to the tests.

Hmmm I dunno...

What I do know is that currently there is ~9.5MB of code sitting under iris.tests 😟

@ajdawson
Copy link
Member

My line of thought was that if iris has been installed as a package then it's most likely a pucker packaged release, and it should just work.

I can buy that line of thought. I guess packaging the tests used to make a lot of sense before conda and wheels were prevalent and we used to have to install from source ourselves. I will not object to proceeding with this work.

@pp-mo
Copy link
Member

pp-mo commented Oct 14, 2019

I'm 👍 on this in principle, but we should take the opportunity improve the "how to" descriptions for new developers : At present the "Developer's Guide" assumes a distressing level of proficiency, E.G. it lays out how to write tests + where to put them without ever explaining "how to run the tests".
Possibly, "setup.py tests" still works, or not, but that was never well signposted, or even clearly documented in the setup.py command-line help.

@rcomer
Copy link
Member

rcomer commented Oct 14, 2019

Possibly, "setup.py tests" still works

It ran last time I tried, though I had failures in places I hadn't touched. I also got failures because iris.std_names.py didn't get regenerated. It hadn't so far occurred to me to try other ways of running the tests. So, as well as new developers, I'd benefit from an update to these docs.

@trexfeathers
Copy link
Contributor

@SciTools/peloton are all in favour of this 👍

We acknowledge that for certain cases it makes it more difficult to test that Iris is working in an environment into which it is installed, but for those cases it's still possible to run the tests in ANY environment - PROVIDING THE tests DIRECTORY IS NOT IN THE lib DIRECTORY (this is also a big argument for keeping the lib directory).

@lbdreyer
Copy link
Member

lbdreyer commented Oct 18, 2023

I very regularly run the tests in freshly created environments to ensure that iris is functional. Notably, not all the tests pass (!) but this is usually due to image comparison failures or slightly different string formatting or something else caused by different versions of dependencies in my installed environment. Nonetheless it is helpful to get that assurance that the environment works as expected, after confirming any test failures are okay.

Everytime I run the tests in a conda environment, I think of this issue and how much of a pain it would be if the tests weren't there in the install. I could go searching the repo for the tests and download them when I want to run the test in my environment but this would be a bit cumbersome.

So unless the tests get much bigger such that the install of iris is unreasonably large, I would prefer we kept them

@bjlittle
Copy link
Member Author

Cool, let's maintain packaging the tests within iris, as there is genuine benefit in doing so.

Thanks everyone for chipping in, much appreciated 😄

@bjlittle bjlittle closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2023
Technical Debt automation moved this from To do to Done Oct 18, 2023
@rcomer
Copy link
Member

rcomer commented Oct 18, 2023

After a git clean

$ du -sch lib/iris/tests/* | sort -h
4.0K	lib/iris/tests/idiff.py
<snip>
2.0M	lib/iris/tests/unit
17M	lib/iris/tests/stock_arrays.npz
37M	lib/iris/tests/results
57M	total

So the bulk of the "tests" are the results directory and the stock arrays file. Those could potentially be moved into the test data repo if we wanted to trim some weight?

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

No branches or pull requests

6 participants