Shift slow test related pytest hooks in lower level conftest.py #578
Conversation
| test_method_name = report.nodeid.split('::')[-1] | ||
| if test_method_name == "test_spectrum": | ||
| # Just making it work for this commit. Will be changes subsequently | ||
| with open(os.path.join("/tmp/plots", tardis.__githash__[0:7], |
There was a problem hiding this comment.
don't use the png file as a workaround - directly encode to base64 binary thingie
| # generated during execution. | ||
| tempdir_session = tempfile.mkdtemp() | ||
| config.option.tempdir = tempdir_session | ||
|
|
||
| html_file = tempfile.NamedTemporaryFile(delete=False) | ||
| # Html test report will be generated at this filepath by pytest-html plugin | ||
| config.option.htmlpath = html_file.name |
There was a problem hiding this comment.
A tempdir is added in pytest_configure. Would it be better to declare temporary html file inside tempdir now ?
There was a problem hiding this comment.
So pytest-html requires config.option.htmlpath to be set to generate a report?
In any case, I'd store the html file handle in the config, too. Then you don't have to worry about deleting the temp file because it should go when config goes out of scope.
There was a problem hiding this comment.
Yes, pytest-html plugin wants a filepath (unicode or string) in config.option.htmlpath.
There was a problem hiding this comment.
Okay, well I'd say just store the tmpfile in config.option.html
dbe6c33 to
5b11f33
Compare
| config.option.htmlpath = html_file.name | ||
|
|
||
|
|
||
| @remote_data |
There was a problem hiding this comment.
I'm not sure that works and I think that's not how this is supposed to to work.
There was a problem hiding this comment.
It is working, here is the report which I generated this morning using this code.
There was a problem hiding this comment.
and if you remove @remote_data it stops working?
and you have to add --remote-data in the commandline or it won't work?
There was a problem hiding this comment.
I have to do both the things simultaneously to make it work.
|
I have some comments on |
- They conflicted with astropy's test plugins which were imported as *.
| def pytest_unconfigure(config): | ||
| # Html report created by pytest-html plugin is read here, uploaded to | ||
| # dokuwiki and finally deleted. | ||
| if dokuwiki_available: |
There was a problem hiding this comment.
You added a check to pytest_configure to check if we are running the integration tests or not. Please do the same for pytest_unconfigure
|
http://opensupernova.org/~karandesai96/integration/doku.php?id=reports:52115b2 Is the report produced with the latest commit. I'm fine with merging. |
…is-sn#578) * Move slow test related methods to lower level conftest.py - They conflicted with astropy's test plugins which were imported as *. * Use session wide temp director to store plots. * Use remote_data marker on pytest_unconfigure. * Shift unpacking of integration tests config in pytest_configure. * Cosmetic change - remove unused imports from conftest.py * Add a check in pytest_unconfigure for integration tests.
There were two primary issues with infrastructure of integration tests developed till now:
--htmlcommand line option to specify path to save the report locally. But with the help of tempfile module, and also pytest hooks:pytest_configureandpytest_unconfiguredeclared in top level conftest.py, the report was saved in a temp file, uploaded to dokuwiki and finally deleted.*in top level conftest.py and the configure-unconfigure hooks I declared, actually had overwritten the former ones. This loosened the protection of@remote_datamarker provided by astropy test helpers./tmp/plots/w7/tardis.__githash__. I usedos.rmdirto cleanup the temp directory. Butos.rmdircannot remove non empty directories. Hence running multiple tests successively on same commit required manual deletion of that directory, else the tests would raiseOSError.mkdtemp()method of tempfile module. Duringpytest_unconfigure, recursive deletion was performed usingshutilmodule.