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

Make tests work from any directory and functional without special runner script #124

Merged
merged 2 commits into from
Dec 5, 2017

Conversation

llimeht
Copy link
Contributor

@llimeht llimeht commented Nov 2, 2017

Test resources are in the same directory (or a child directory) of the test code, so the code can locate the resources itself rather than relying on the tests being run from a certain location. This PR adds a trivial function to find the necessary data files for the tests.

Making the tests more robust to the current working directory allows the tests to be used against installed code rather than the current checkout. These changes also allow automated test discovery to work and so tools like py.test can run all (or a selection of) tests at once, without the need for a custom test runner script; a pytest.ini configuration is included to facilitate this. For example, in the Debian packaging, we can now run the tests against the compiled version by running nothing more than py.test.

(This PR builds on top of #122 and so the commit in that PR will be shown here too)

@pkienzle
Copy link
Contributor

pkienzle commented Nov 2, 2017

If we want to be able to run tests against installed code, shouldn't we move the test code into the src tree?

@pkienzle
Copy link
Contributor

pkienzle commented Nov 2, 2017

Since you no longer need to be in the test directory when running the test, you can update test/run_one.py to drop the chdir.

NB, "find" is a bit of a misnomer, since the output targets don't actually exist to be found when you are resolving their path. Can't think of a better name at the moment.

Copy link
Contributor

@pkienzle pkienzle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving with reservations.

It adds a lot of code but it is conceptually cleaner to not change directories before each test. The same goal of testing the against the install base could be accomplished with a simple flag to the test script.

The implied dependence on pytest via the pytest.ini script adds more maintenance burden, and only for testing the installed package. Day-to-day development still needs to use the scripts to set up the in-place runtime environment.

Maybe "setup.py install develop" will work with the reorganized tree, so we can still get the benefit of editing the source and running without a build step without having to go through run.py, and we could use nose or pytest or whatever for day-to-day use.

@llimeht
Copy link
Contributor Author

llimeht commented Nov 3, 2017

As a long term aim, I think it is realistic to remove the custom test discovery and test runner code that currently exists and to get in-place testing to work. It's much better to outsource as much of this as possible to a tool like py.test that will do a better job and doesn't require sasview people to maintain it. I view these changes are a first step towards that goal, with the next step being to enable in-place testing, as you say, although that is like to be a much more invasive patch set and it's not needed just yet for my current goal of getting working Debian packages for sasview.

(Am happy to rename find to something else; naming things is hard! generate_local_resource_name didn't feel right…)

@pkienzle
Copy link
Contributor

pkienzle commented Nov 3, 2017

No objection to the goal. I would have merged, but I'm waiting for your response to #122.

Re: find, we have variously used the following, for more or less related behaviour:

find_app_dir, _find_config_file, find_plugins, load_file_and_schema, initialize_plugins_dir, get_data_path (x5), get_media_path, _get_default_cat_file_dir, resource_dir

I suggest get_data_path, but feel free to leave it as is or choose a different name.

Class variables get inherited and shared between different instantiations
of the class and this means that the current usage of TestFileReader in
utest_generic_file_reader_class.py has side-effects that would cause
subsequent tests to fail.
Test resources are in the same directory (or a child directory) of the test
code so the code can locate the resources itself rather than relying on the
tests being run from a certain location. This allows the tests to be used to
test the installed code rather than the current checkout and also allows
automated test discovery with tools like py.test to be used, running all
tests at once without a runner.
@pkienzle pkienzle merged commit f53d684 into SasView:master Dec 5, 2017
@llimeht llimeht deleted the tmp/standalone-utests branch January 19, 2018 22:34
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.

None yet

2 participants