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

Disable caching of test results #23

Merged
merged 1 commit into from
Jul 24, 2020
Merged

Disable caching of test results #23

merged 1 commit into from
Jul 24, 2020

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Jun 28, 2020

Description

Disable caching of test results to prevent users trying to write into flight directory if tests fail running on installed package.

I think this impacts python setup.py test also (but not 100% sure) but not direct pytest (which doesn't work for namespace packages unfortunately).

Testing

  • [N/A] Passes unit tests (no tests)
  • Functional testing

Running run_testr as user aldcroft in ska3-shiny on HEAD produces pytest warnings for every unit test because it is trying to write to cache info to /proj/sot/ska3/shiny. I applied this patch as a hot-fix to ska3-shiny on HEAD and confirmed that all those warnings go away and tests still run as expected.

@javierggt
Copy link
Contributor

Hi @taldcroft, I am assuming you read my last comment in PR 21. Wouldn't it be possible to pass this in TESTR_PYTEST_ARGS whenever it is needed?

@taldcroft
Copy link
Member Author

Per my comment in #21, while I'm fine with having environment var hooks for behaviors which can be useful in running package tests, in general I'm thinking of myself and other users that won't know or think about setting them.

The problem of a .cache directory being written into the installed package area is real (when doing import <pkg>; <pkg>.test()). This .cache found its way into the production ska3-flight when running self tests as aca. Not a disaster, but not a good thing.

kady$ pwd
/proj/sot/ska3/flight/lib/python3.6/site-packages
kady$ ls -l .cache/v/cache/lastfailed 
-rw-r--r-- 1 aca aspect 197 May 30 13:50 .cache/v/cache/lastfailed
kady$ cat .cache/v/cache/lastfailed 
{
  "Ska/engarchive/tests/test_comps.py::test_mups_valve": true,
  "Ska/engarchive/tests/test_sync.py::test_sync[cpe1eng]": true,
  "Ska/engarchive/tests/test_sync.py::test_sync[pcad13eng]": true
}

@taldcroft
Copy link
Member Author

This needed to get a basic run_testr passing on HEAD shiny. I confirmed that it works as expected so merging now.

@taldcroft taldcroft merged commit 2e719d0 into master Jul 24, 2020
@javierggt javierggt mentioned this pull request Dec 7, 2020
@javierggt javierggt deleted the disable-caching branch April 20, 2022 20:28
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