[Scons] Remove "install" target from "check_for_pytest" #1434
Conversation
The `check_for_pytest` depends on `check_for_ruamel_yaml`
the pytest package is required on `install` phase and it results in
the Cantera installation fail if pytest absent
even if tests were not required.
This patch swap `check_for_pytest` and `check_for_ruamel_yaml`
dependency order.
The explicit `test-python-convert`, `test-python` targets are dropped
from `check_for_ruamel_yaml` because the `check_for_pytest`
includes `target.startswith("test-python")`.
Signed-off-by: Sergey Torokhov <torokhov-s-a@yandex.ru>
There was a problem hiding this comment.
Thanks, @band-a-prend. Changes look good to me - some tests appear to be failing for unrelated upstream reasons - I’ll retrigger them, expecting them to pass eventually.
| @@ -1697,17 +1697,17 @@ ruamel_min_version = parse_version('0.15.34') | |||
| # Minimum pytest version assumed based on Ubuntu 20.04 | |||
| pytest_min_version = parse_version("4.6.9") | |||
|
|
|||
| # Pytest is required only to test the Python module | |||
| check_for_pytest = any( | |||
| target.startswith("test-python") for target in COMMAND_LINE_TARGETS | |||
There was a problem hiding this comment.
the command line target test should also trigger this check.
There was a problem hiding this comment.
this is correct - sorry for jumping the gun on this
There was a problem hiding this comment.
@speth
If cantera built without "python_full" or "python_minimal" should then "test" trigger the check_for_pytest? Is pytest required just for C++ library and Fortran wrapper testing?
There was a problem hiding this comment.
No, of course not. But test causes the Python test suite to be executed when the Python module is installed, so this check does need to happen in that case.
It's not clear from this line alone, but if you look further down where this test is actually handled, you'll see that it happens in a block that checks that env['python_package'] != 'none'.
Addendum to Cantera#1434; update ensures that checks are run for all tests of the test suite that involve pytest.
Addendum to #1434; update ensures that checks are run for all tests of the test suite that involve pytest.
Changes proposed in this pull request
The
check_for_pytestdepends oncheck_for_ruamel_yamlthe pytest package is required oninstallphase and it results in the Cantera installation fail if pytest absent even if tests were not required.This patch swap
check_for_pytestandcheck_for_ruamel_yamldependency order.The explicit
test-python-convert,test-pythontargets are dropped fromcheck_for_ruamel_yamlbecause thecheck_for_pytestincludestarget.startswith("test-python").If applicable, fill in the issue number this pull request is fixing
See commit 72734b1#r97347952 discussion
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build&scons test) and unit tests address code coverage