Closed pllim closed 3 years ago
I added @bsipocz as a reviewer since she expressed interest to review. FYI and thanks!
Here is e.g. what I mean: when the parent is sunpy, I think this test header, and possibly other stuff as well, don't seem to be right. I recall this came up already in the header, or test runner, or somewhere (e.g. astropy should not be listed in the top, but rather as a dependency below, along with dependencies declared for "packagename", rather than a hard wired list). Also, I don't see any indications that the extra context is being tested, so with this matrix we only test that it doesn't error out?:
============================= test session starts ==============================
platform linux -- Python 3.8.6, pytest-6.2.1, py-1.10.0, pluggy-0.13.1
cachedir: .tox/py38-test/.pytest_cache
Running tests with Astropy version 4.2.
Running tests in packagename docs.
Date: 2021-01-13T19:45:18
Platform: Linux-5.4.0-1032-azure-x86_64-with-glibc2.2.5
Executable: /home/runner/work/package-template/test/packagename/.tox/py38-test/bin/python
Full Python Version:
3.8.6 (default, Oct 28 2020, 13:08:18)
[GCC 7.5.0]
encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15
Package versions:
Numpy: 1.19.5
Scipy: not available
Matplotlib: not available
h5py: not available
Pandas: not available
Using Astropy options: remote_data: none.
rootdir: /home/runner/work/package-template/test/packagename, configfile: setup.cfg
plugins: hypothesis-6.0.1, arraydiff-0.3, doctestplus-0.8.0, astropy-header-0.1.2, remotedata-0.3.2, cov-2.10.1, filter-subpackage-0.1.1, openfiles-0.5.0
collected 3 items
test header
Yes, that is a separate issue already explained at https://github.com/astropy/pytest-astropy-header/issues/15#issuecomment-763247031 .
indications that the extra context is being tested
I am not sure what you mean. Compared to https://travis-ci.com/github/astropy/package-template/jobs/430750636 (where the parent is sunpy also), there are only 3 tests. For Actions, I run the codestyle and build_docs first and you can see the results if you expand the relevant steps before the tests.
@pllim - This PR is certainly replaces everything that we did with travis, and more. All I'm saying is that I don't see whether the extra contexts are being tested at all, beyond checking that no errors are raised using them. Probably I should have just opened a separate issue for it, as it clearly goes beyond this PR.
Yes, the matrix itself definitely can be improved for future PR. Shall we merge then? 😸
Oh, sure, I didn't mean that to be a blocker here more just as something I noticed, as this PR already improves the current status.
Fix #499 .
Proof of concept as follows:
Some notes:
render
job is not in a matrix becauseif
inside a step isn't respected in a matrixed workflow, and you don't want to upload the artifact unnecessarily.checkout
action is pretty nice as you don't need special SSH key by using it. But you do need a special token that allows "update github action workflows" and I already set that up for you. Would be nice if we can replace it with some org level token but I don't have access to that (some site says that is not even possible, so I am not sure).Note to self: