From c6cb4fd108125c4a95fbf2066875290c9509da01 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 16:36:06 -0700 Subject: [PATCH 01/17] Add support for an xfails file, and allow the files to be specified with a flag --- conftest.py | 57 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/conftest.py b/conftest.py index e0453e40..d9ca5628 100644 --- a/conftest.py +++ b/conftest.py @@ -47,7 +47,17 @@ def pytest_addoption(parser): parser.addoption( "--ci", action="store_true", - help="run just the tests appropiate for CI", + help="run just the tests appropriate for CI", + ) + parser.addoption( + "--skips-file", + action="store", + help="file with tests to skip. Defaults to skips.txt" + ) + parser.addoption( + "--xfails-file", + action="store", + help="file with tests to skip. Defaults to xfails.txt" ) @@ -82,26 +92,49 @@ def xp_has_ext(ext: str) -> bool: return False -skip_ids = [] -skips_path = Path(__file__).parent / "skips.txt" -if skips_path.exists(): - with open(skips_path) as f: - for line in f: - if line.startswith("array_api_tests"): - id_ = line.strip("\n") - skip_ids.append(id_) +def pytest_collection_modifyitems(config, items): + skips_file = skips_path = config.getoption('--skips-file') + if skips_file is None: + skips_file = Path(__file__).parent / "skips.txt" + if skips_file.exists(): + skips_path = skips_file + skip_ids = [] + if skips_path: + with open(skips_path) as f: + for line in f: + if line.startswith("array_api_tests"): + id_ = line.strip("\n") + skip_ids.append(id_) + + xfails_file = xfails_path = config.getoption('--xfails-file') + if xfails_file is None: + xfails_file = Path(__file__).parent / "xfails.txt" + if xfails_file.exists(): + xfails_path = xfails_file + + xfail_ids = [] + if xfails_path: + with open(xfails_path) as f: + for line in f: + if line.startswith("array_api_tests"): + id_ = line.strip("\n") + xfail_ids.append(id_) -def pytest_collection_modifyitems(config, items): disabled_exts = config.getoption("--disable-extension") disabled_dds = config.getoption("--disable-data-dependent-shapes") ci = config.getoption("--ci") for item in items: markers = list(item.iter_markers()) - # skip if specified in skips.txt + # skip if specified in skips file for id_ in skip_ids: if item.nodeid.startswith(id_): - item.add_marker(mark.skip(reason="skips.txt")) + item.add_marker(mark.skip(reason="skips file")) + break + # xfail if specified in xfails file + for id_ in xfail_ids: + if item.nodeid.startswith(id_): + item.add_marker(mark.xfails(reason="xfails file")) break # skip if disabled or non-existent extension ext_mark = next((m for m in markers if m.name == "xp_extension"), None) From 24f67f8db06d0d112c6a081af226e02baaafcca4 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 18:21:10 -0700 Subject: [PATCH 02/17] Don't skip test_has_names with --ci --- array_api_tests/test_has_names.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/array_api_tests/test_has_names.py b/array_api_tests/test_has_names.py index d9194d82..3c5c3263 100644 --- a/array_api_tests/test_has_names.py +++ b/array_api_tests/test_has_names.py @@ -9,6 +9,8 @@ from .stubs import (array_attributes, array_methods, category_to_funcs, extension_to_funcs, EXTENSIONS) +pytestmark = pytest.mark.ci + has_name_params = [] for ext, stubs in extension_to_funcs.items(): for stub in stubs: From 21d6b9f62cd219c3ec4a2297dcfcd3151246b11d Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 18:38:34 -0700 Subject: [PATCH 03/17] Be more lax about id names in xfail and skips files Also include the file name in the reason. --- conftest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/conftest.py b/conftest.py index d9ca5628..9b4947ed 100644 --- a/conftest.py +++ b/conftest.py @@ -128,13 +128,13 @@ def pytest_collection_modifyitems(config, items): markers = list(item.iter_markers()) # skip if specified in skips file for id_ in skip_ids: - if item.nodeid.startswith(id_): - item.add_marker(mark.skip(reason="skips file")) + if id_ in item.nodeid: + item.add_marker(mark.skip(reason=f"skips file ({skips_file})")) break # xfail if specified in xfails file for id_ in xfail_ids: - if item.nodeid.startswith(id_): - item.add_marker(mark.xfails(reason="xfails file")) + if id_ in item.nodeid: + item.add_marker(mark.xfail(reason=f"xfails file ({xfails_file})")) break # skip if disabled or non-existent extension ext_mark = next((m for m in markers if m.name == "xp_extension"), None) From f66128471fbeb7ce4f306f90ccec2d2cdc43ab9d Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 18:53:47 -0700 Subject: [PATCH 04/17] Move the NumPy skips on CI to a skips file --- .github/workflows/numpy.yml | 49 +------------------------------------ numpy-skips.txt | 41 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 48 deletions(-) create mode 100644 numpy-skips.txt diff --git a/.github/workflows/numpy.yml b/.github/workflows/numpy.yml index 74eeccf4..5cced17c 100644 --- a/.github/workflows/numpy.yml +++ b/.github/workflows/numpy.yml @@ -28,51 +28,4 @@ jobs: env: ARRAY_API_TESTS_MODULE: numpy.array_api run: | - # Skip testing functions with known issues - cat << EOF >> skips.txt - - # copy not implemented - array_api_tests/test_creation_functions.py::test_asarray_arrays - # https://github.com/numpy/numpy/issues/20870 - array_api_tests/test_data_type_functions.py::test_can_cast - # The return dtype for trace is not consistent in the spec - # https://github.com/data-apis/array-api/issues/202#issuecomment-952529197 - array_api_tests/test_linalg.py::test_trace - # waiting on NumPy to allow/revert distinct NaNs for np.unique - # https://github.com/numpy/numpy/issues/20326#issuecomment-1012380448 - array_api_tests/test_set_functions.py - - # https://github.com/numpy/numpy/issues/21373 - array_api_tests/test_array_object.py::test_getitem - - # missing copy arg - array_api_tests/test_signatures.py::test_func_signature[reshape] - - # https://github.com/numpy/numpy/issues/21211 - array_api_tests/test_special_cases.py::test_iop[__iadd__(x1_i is -0 and x2_i is -0) -> -0] - # https://github.com/numpy/numpy/issues/21213 - array_api_tests/test_special_cases.py::test_iop[__ipow__(x1_i is -infinity and x2_i > 0 and not (x2_i.is_integer() and x2_i % 2 == 1)) -> +infinity] - array_api_tests/test_special_cases.py::test_iop[__ipow__(x1_i is -0 and x2_i > 0 and not (x2_i.is_integer() and x2_i % 2 == 1)) -> +0] - # noted diversions from spec - array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity] - array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is +infinity and isfinite(x2_i) and x2_i < 0) -> -infinity] - array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is -infinity and isfinite(x2_i) and x2_i > 0) -> -infinity] - array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is -infinity and isfinite(x2_i) and x2_i < 0) -> +infinity] - array_api_tests/test_special_cases.py::test_binary[floor_divide(isfinite(x1_i) and x1_i > 0 and x2_i is -infinity) -> -0] - array_api_tests/test_special_cases.py::test_binary[floor_divide(isfinite(x1_i) and x1_i < 0 and x2_i is +infinity) -> -0] - array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity] - array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i < 0) -> -infinity] - array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i > 0) -> -infinity] - array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i < 0) -> +infinity] - array_api_tests/test_special_cases.py::test_binary[__floordiv__(isfinite(x1_i) and x1_i > 0 and x2_i is -infinity) -> -0] - array_api_tests/test_special_cases.py::test_binary[__floordiv__(isfinite(x1_i) and x1_i < 0 and x2_i is +infinity) -> -0] - array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity] - array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i < 0) -> -infinity] - array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i > 0) -> -infinity] - array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i < 0) -> +infinity] - array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(isfinite(x1_i) and x1_i > 0 and x2_i is -infinity) -> -0] - array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(isfinite(x1_i) and x1_i < 0 and x2_i is +infinity) -> -0] - - EOF - - pytest -v -rxXfE --ci + pytest -v -rxXfE --ci --skips-file numpy-skips.txt diff --git a/numpy-skips.txt b/numpy-skips.txt new file mode 100644 index 00000000..9f6c32aa --- /dev/null +++ b/numpy-skips.txt @@ -0,0 +1,41 @@ +# copy not implemented +array_api_tests/test_creation_functions.py::test_asarray_arrays +# https://github.com/numpy/numpy/issues/20870 +array_api_tests/test_data_type_functions.py::test_can_cast +# The return dtype for trace is not consistent in the spec +# https://github.com/data-apis/array-api/issues/202#issuecomment-952529197 +array_api_tests/test_linalg.py::test_trace +# waiting on NumPy to allow/revert distinct NaNs for np.unique +# https://github.com/numpy/numpy/issues/20326#issuecomment-1012380448 +array_api_tests/test_set_functions.py + +# https://github.com/numpy/numpy/issues/21373 +array_api_tests/test_array_object.py::test_getitem + +# missing copy arg +array_api_tests/test_signatures.py::test_func_signature[reshape] + +# https://github.com/numpy/numpy/issues/21211 +array_api_tests/test_special_cases.py::test_iop[__iadd__(x1_i is -0 and x2_i is -0) -> -0] +# https://github.com/numpy/numpy/issues/21213 +array_api_tests/test_special_cases.py::test_iop[__ipow__(x1_i is -infinity and x2_i > 0 and not (x2_i.is_integer() and x2_i % 2 == 1)) -> +infinity] +array_api_tests/test_special_cases.py::test_iop[__ipow__(x1_i is -0 and x2_i > 0 and not (x2_i.is_integer() and x2_i % 2 == 1)) -> +0] +# noted diversions from spec +array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity] +array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is +infinity and isfinite(x2_i) and x2_i < 0) -> -infinity] +array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is -infinity and isfinite(x2_i) and x2_i > 0) -> -infinity] +array_api_tests/test_special_cases.py::test_binary[floor_divide(x1_i is -infinity and isfinite(x2_i) and x2_i < 0) -> +infinity] +array_api_tests/test_special_cases.py::test_binary[floor_divide(isfinite(x1_i) and x1_i > 0 and x2_i is -infinity) -> -0] +array_api_tests/test_special_cases.py::test_binary[floor_divide(isfinite(x1_i) and x1_i < 0 and x2_i is +infinity) -> -0] +array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity] +array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i < 0) -> -infinity] +array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i > 0) -> -infinity] +array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i < 0) -> +infinity] +array_api_tests/test_special_cases.py::test_binary[__floordiv__(isfinite(x1_i) and x1_i > 0 and x2_i is -infinity) -> -0] +array_api_tests/test_special_cases.py::test_binary[__floordiv__(isfinite(x1_i) and x1_i < 0 and x2_i is +infinity) -> -0] +array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity] +array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i < 0) -> -infinity] +array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i > 0) -> -infinity] +array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(x1_i is -infinity and isfinite(x2_i) and x2_i < 0) -> +infinity] +array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(isfinite(x1_i) and x1_i > 0 and x2_i is -infinity) -> -0] +array_api_tests/test_special_cases.py::test_iop[__ifloordiv__(isfinite(x1_i) and x1_i < 0 and x2_i is +infinity) -> -0] From 153c9fa851276257ac2e923a13a268e09abcc297 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 18:54:10 -0700 Subject: [PATCH 05/17] Be more generous about what lines are allowed in a skips/xfail file --- conftest.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/conftest.py b/conftest.py index 9b4947ed..14a09f5b 100644 --- a/conftest.py +++ b/conftest.py @@ -117,9 +117,10 @@ def pytest_collection_modifyitems(config, items): if xfails_path: with open(xfails_path) as f: for line in f: - if line.startswith("array_api_tests"): - id_ = line.strip("\n") - xfail_ids.append(id_) + if not line.strip() or line.startswith('#'): + continue + id_ = line.strip("\n") + xfail_ids.append(id_) disabled_exts = config.getoption("--disable-extension") disabled_dds = config.getoption("--disable-data-dependent-shapes") From 373d05f0bb54258bf2467a38cd07f7ecec70654f Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 18:54:40 -0700 Subject: [PATCH 06/17] Update the README for skips/xfails files --- README.md | 64 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 9eebc397..d1accf2d 100644 --- a/README.md +++ b/README.md @@ -178,13 +178,22 @@ By default, tests for the optional Array API extensions such as will be skipped if not present in the specified array module. You can purposely skip testing extension(s) via the `--disable-extension` option. -#### Skip test cases +#### Skip or XFAIL test cases -Test cases you want to skip can be specified in a `skips.txt` file in the root -of this repository, e.g.: +Test cases you want to skip can be specified in a skips or XFAILS file. The +difference between skip and XFAIL is that XFAIL tests are still run and +reported as XPASS if they pass. + +By default, the skips and xfails files are `skips.txt` and `fails.txt` in the root +of this repository, but any file can be specified with the `--skips-file` and +`--xfails-file` command line flags. + +The files should list the test ids to be skipped/xfailed. Empty lines and +lines starting with `#` are ignored. The test id can be any substring of the +test ids to skip/xfail. ``` -# ./skips.txt +# skips.txt or xfails.txt # Line comments can be denoted with the hash symbol (#) # Skip specific test case, e.g. when argsort() does not respect relative order @@ -200,29 +209,42 @@ array_api_tests/test_add[__iadd__(x, s)] array_api_tests/test_set_functions.py ``` -For GitHub Actions, you might like to keep everything in the workflow config -instead of having a seperate `skips.txt` file, e.g.: +Here is an example GitHub Actions workflow file, where the xfails are stored +in `array-api-tests.xfails.txt` in the base of the `your-array-library` repo. + +Note that this uses `-o xfail_strict=True`. This causes XPASS tests (XFAIL +tests that actually pass) to fail the test suite. If you don't want this +behavior, you can remove it, or use `--skips-file` instead of `--xfails-file`. ```yaml # ./.github/workflows/array_api.yml -... - ... - - name: Run the test suite +jobs: + tests: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ['3.8', '3.9', '3.10', '3.11'] + + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + path: your-array-library + + - name: Checkout array-api-tests + uses: actions/checkout@v3 + with: + repository: data-apis/array-api-tests + submodules: 'true' + path: array-api-tests + + - name: Run the array API test suite env: ARRAY_API_TESTS_MODULE: your.array.api.namespace run: | - # Skip test cases with known issues - cat << EOF >> skips.txt - - # Comments can still work here - array_api_tests/test_sorting_functions.py::test_argsort - array_api_tests/test_add[__iadd__(x1, x2)] - array_api_tests/test_add[__iadd__(x, s)] - array_api_tests/test_set_functions.py - - EOF - - pytest -v -rxXfE --ci + export PYTHONPATH="${GITHUB_WORKSPACE}/array-api-compat" + cd ${GITHUB_WORKSPACE}/array-api-tests + pytest -v -rxXfE --ci -o xfail_strict=True --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt ``` #### Max examples From 68470c64886e71ccfc85ffe743fc36fcfa937fc7 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 2 Jan 2023 19:03:21 -0700 Subject: [PATCH 07/17] Fix the CI example in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d1accf2d..635d3c89 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,7 @@ jobs: run: | export PYTHONPATH="${GITHUB_WORKSPACE}/array-api-compat" cd ${GITHUB_WORKSPACE}/array-api-tests - pytest -v -rxXfE --ci -o xfail_strict=True --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt + pytest -v -rxXfE --ci -o xfail_strict=True --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt array_api_tests/ ``` #### Max examples From 8e2bff8710d7d69ac163295a19b9f55efdc1dbe0 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Thu, 16 Feb 2023 16:44:33 -0700 Subject: [PATCH 08/17] Add a warning about the flakyness of XFAILS in the README --- README.md | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 635d3c89..eb542324 100644 --- a/README.md +++ b/README.md @@ -247,14 +247,37 @@ jobs: pytest -v -rxXfE --ci -o xfail_strict=True --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt array_api_tests/ ``` +> **Warning** +> XFAIL tests that use Hypothesis (basically every test in the test suite except +> those in test_has_names.py) can be flaky, due to the fact that Hypothesis +> might not always run the test with an input that causes the test to fail. +> There are several ways to avoid this problem: +> +> - Increase the maximum number of examples, e.g., by adding `--max-examples +> 1000` to the test command (the default is `100`, see below). This will +> make it more likely that the failing case will be found. +> - Don't use `-o xfail_strict=True`. This will make it so that if an XFAIL +> test passes, it will alert you in the test summary but will not cause the +> test run to register as failed. +> - Use skips instead of XFAILS. The difference between XFAIL and skip is that +> a skipped test is never run at all, whereas an XFAIL test is always run +> but ignored if it fails. +> - Save the [Hypothesis examples +> database](https://hypothesis.readthedocs.io/en/latest/database.html) +> persistently on CI. That way as soon as a run finds one failing example, +> it will always re-run future runs with that example. But note that the +> Hypothesis examples database may be cleared when a new version of +> Hypothesis or the test suite is released. + #### Max examples The tests make heavy use [Hypothesis](https://hypothesis.readthedocs.io/en/latest/). You can configure -how many examples are generated using the `--max-examples` flag, which defaults -to 100. Lower values can be useful for quick checks, and larger values should -result in more rigorous runs. For example, `--max-examples 10_000` may find bugs -where default runs don't but will take much longer to run. +how many examples are generated using the `--max-examples` flag, which +defaults to `100`. Lower values can be useful for quick checks, and larger +values should result in more rigorous runs. For example, `--max-examples +10_000` may find bugs where default runs don't but will take much longer to +run. ## Contributing From 82cc470b5f98d3d26464f0ce84b028f396f17e27 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Thu, 16 Feb 2023 16:45:30 -0700 Subject: [PATCH 09/17] Fix Markdown formatting --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index eb542324..2c0816e6 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,7 @@ jobs: ``` > **Warning** +> > XFAIL tests that use Hypothesis (basically every test in the test suite except > those in test_has_names.py) can be flaky, due to the fact that Hypothesis > might not always run the test with an input that causes the test to fail. From 234e9aac87b98bbbaf88700a1000df941b6e57f1 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Thu, 16 Feb 2023 17:46:02 -0700 Subject: [PATCH 10/17] Add note about max examples --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2c0816e6..e3efad9d 100644 --- a/README.md +++ b/README.md @@ -256,7 +256,8 @@ jobs: > > - Increase the maximum number of examples, e.g., by adding `--max-examples > 1000` to the test command (the default is `100`, see below). This will -> make it more likely that the failing case will be found. +> make it more likely that the failing case will be found, but it will also +> make the tests take longer to run. > - Don't use `-o xfail_strict=True`. This will make it so that if an XFAIL > test passes, it will alert you in the test summary but will not cause the > test run to register as failed. From d7b457ab0686e0baa7a53204f3aed884fa028f41 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Thu, 16 Feb 2023 19:38:31 -0700 Subject: [PATCH 11/17] Decrease the recommended number of max examples for CI --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e3efad9d..a5010bec 100644 --- a/README.md +++ b/README.md @@ -255,7 +255,7 @@ jobs: > There are several ways to avoid this problem: > > - Increase the maximum number of examples, e.g., by adding `--max-examples -> 1000` to the test command (the default is `100`, see below). This will +> 200` to the test command (the default is `100`, see below). This will > make it more likely that the failing case will be found, but it will also > make the tests take longer to run. > - Don't use `-o xfail_strict=True`. This will make it so that if an XFAIL From 44c7498db3dbfe9dd871d98478e641ff2e8f5b4b Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Thu, 16 Feb 2023 19:46:40 -0700 Subject: [PATCH 12/17] Warn if a skip or xfail from a file doesn't correspond to any tests --- conftest.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/conftest.py b/conftest.py index 14a09f5b..e3522062 100644 --- a/conftest.py +++ b/conftest.py @@ -1,5 +1,6 @@ from functools import lru_cache from pathlib import Path +import warnings from hypothesis import settings from pytest import mark @@ -125,18 +126,27 @@ def pytest_collection_modifyitems(config, items): disabled_exts = config.getoption("--disable-extension") disabled_dds = config.getoption("--disable-data-dependent-shapes") ci = config.getoption("--ci") - for item in items: - markers = list(item.iter_markers()) - # skip if specified in skips file - for id_ in skip_ids: + # skip if specified in skips file + for id_ in skip_ids: + for item in items: if id_ in item.nodeid: item.add_marker(mark.skip(reason=f"skips file ({skips_file})")) break - # xfail if specified in xfails file - for id_ in xfail_ids: + else: + warnings.warn(f"Skip ID from {skips_file} doesn't appear to correspond to any tests: {id_!r}") + + # xfail if specified in xfails file + for id_ in xfail_ids: + for item in items: if id_ in item.nodeid: item.add_marker(mark.xfail(reason=f"xfails file ({xfails_file})")) break + else: + warnings.warn(f"XFAIL ID from {xfails_file} doesn't appear to correspond to any tests: {id_!r}") + + for item in items: + markers = list(item.iter_markers()) + # skip if disabled or non-existent extension ext_mark = next((m for m in markers if m.name == "xp_extension"), None) if ext_mark is not None: From df7ffe36940d40a2f4573ba1f94cfed89e651c99 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 20 Feb 2023 17:13:53 -0700 Subject: [PATCH 13/17] Remove recommendation to use -o xfail_strict=True --- README.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a5010bec..21e4a212 100644 --- a/README.md +++ b/README.md @@ -212,9 +212,13 @@ array_api_tests/test_set_functions.py Here is an example GitHub Actions workflow file, where the xfails are stored in `array-api-tests.xfails.txt` in the base of the `your-array-library` repo. -Note that this uses `-o xfail_strict=True`. This causes XPASS tests (XFAIL -tests that actually pass) to fail the test suite. If you don't want this -behavior, you can remove it, or use `--skips-file` instead of `--xfails-file`. +If you want, you can use `-o xfail_strict=True`, which causes XPASS tests (XFAIL +tests that actually pass) to fail the test suite. However, be aware that +XFAILures can be flaky (see below, so this may not be a good idea unless you +use some other mitigation of such flakyness). + +If you don't want this behavior, you can remove it, or use `--skips-file` +instead of `--xfails-file`. ```yaml # ./.github/workflows/array_api.yml @@ -244,7 +248,7 @@ jobs: run: | export PYTHONPATH="${GITHUB_WORKSPACE}/array-api-compat" cd ${GITHUB_WORKSPACE}/array-api-tests - pytest -v -rxXfE --ci -o xfail_strict=True --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt array_api_tests/ + pytest -v -rxXfE --ci --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt array_api_tests/ ``` > **Warning** From 7c8bbad55bbaed6c32d3242aa8fb8e4278b72ba3 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Fri, 10 Mar 2023 13:52:24 -0700 Subject: [PATCH 14/17] Fix skips and xfails files using ~ --- conftest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index e3522062..7dc5d857 100644 --- a/conftest.py +++ b/conftest.py @@ -1,6 +1,7 @@ from functools import lru_cache from pathlib import Path import warnings +import os from hypothesis import settings from pytest import mark @@ -102,7 +103,7 @@ def pytest_collection_modifyitems(config, items): skip_ids = [] if skips_path: - with open(skips_path) as f: + with open(os.path.expanduser(skips_path)) as f: for line in f: if line.startswith("array_api_tests"): id_ = line.strip("\n") @@ -116,7 +117,7 @@ def pytest_collection_modifyitems(config, items): xfail_ids = [] if xfails_path: - with open(xfails_path) as f: + with open(os.path.expanduser(xfails_path)) as f: for line in f: if not line.strip() or line.startswith('#'): continue From fe878b0562b3d7516392e92d6afc5944007e9c4d Mon Sep 17 00:00:00 2001 From: Matthew Barber Date: Tue, 14 Mar 2023 13:17:31 +0000 Subject: [PATCH 15/17] Fix wildcard skip/xfail ids not marking all child tests --- conftest.py | 56 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/conftest.py b/conftest.py index 01696ceb..8dc7493a 100644 --- a/conftest.py +++ b/conftest.py @@ -129,30 +129,27 @@ def pytest_collection_modifyitems(config, items): id_ = line.strip("\n") xfail_ids.append(id_) + skip_id_matched = {id_: False for id_ in skip_ids} + xfail_id_matched = {id_: False for id_ in xfail_ids} + disabled_exts = config.getoption("--disable-extension") disabled_dds = config.getoption("--disable-data-dependent-shapes") ci = config.getoption("--ci") - # skip if specified in skips file - for id_ in skip_ids: - for item in items: - if id_ in item.nodeid: - item.add_marker(mark.skip(reason=f"skips file ({skips_file})")) - break - else: - warnings.warn(f"Skip ID from {skips_file} doesn't appear to correspond to any tests: {id_!r}") - - # xfail if specified in xfails file - for id_ in xfail_ids: - for item in items: - if id_ in item.nodeid: - item.add_marker(mark.xfail(reason=f"xfails file ({xfails_file})")) - break - else: - warnings.warn(f"XFAIL ID from {xfails_file} doesn't appear to correspond to any tests: {id_!r}") for item in items: markers = list(item.iter_markers()) - + # skip if specified in skips file + for id_ in skip_ids: + if item.nodeid.startswith(id_): + item.add_marker(mark.skip(reason=f"--skips-file ({skips_file})")) + skip_id_matched[id_] = True + break + # xfail if specified in xfails file + for id_ in xfail_ids: + if item.nodeid.startswith(id_): + item.add_marker(mark.xfail(reason=f"--xfails-file ({xfails_file})")) + xfail_id_matched[id_] = True + break # skip if disabled or non-existent extension ext_mark = next((m for m in markers if m.name == "xp_extension"), None) if ext_mark is not None: @@ -186,3 +183,26 @@ def pytest_collection_modifyitems(config, items): reason=f"requires ARRAY_API_TESTS_VERSION=>{min_version}" ) ) + + bad_ids_end_msg = ( + "Note the relevant tests might not of been collected by pytest, or " + "another specified id might have already matched a test." + ) + bad_skip_ids = [id_ for id_, matched in skip_id_matched.items() if not matched] + if bad_skip_ids: + f_bad_ids = "\n".join(f" {id_}" for id_ in bad_skip_ids) + warnings.warn( + f"{len(bad_skip_ids)} ids in skips file don't match any collected tests: \n" + f"{f_bad_ids}\n" + f"(skips file: {skips_file})\n" + f"{bad_ids_end_msg}" + ) + bad_xfail_ids = [id_ for id_, matched in xfail_id_matched.items() if not matched] + if bad_xfail_ids: + f_bad_ids = "\n".join(f" {id_}" for id_ in bad_xfail_ids) + warnings.warn( + f"{len(bad_xfail_ids)} ids in xfails file don't match any collected tests: \n" + f"{f_bad_ids}\n" + f"(xfails file: {xfails_file})\n" + f"{bad_ids_end_msg}" + ) From 1016a0282255007f847d274f2e256253f8231ae8 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Tue, 21 Mar 2023 15:25:17 -0600 Subject: [PATCH 16/17] Fix skip and xfail id matching For some reason the real nodeid has an extra "array-api-tests/" in front which breaks matching if we do a startswith match. "in" would also allow doing simpler matching with just the test name if desired. --- conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 8dc7493a..ca371298 100644 --- a/conftest.py +++ b/conftest.py @@ -140,13 +140,13 @@ def pytest_collection_modifyitems(config, items): markers = list(item.iter_markers()) # skip if specified in skips file for id_ in skip_ids: - if item.nodeid.startswith(id_): + if id_ in item.nodeid: item.add_marker(mark.skip(reason=f"--skips-file ({skips_file})")) skip_id_matched[id_] = True break # xfail if specified in xfails file for id_ in xfail_ids: - if item.nodeid.startswith(id_): + if id_ in item.nodeid: item.add_marker(mark.xfail(reason=f"--xfails-file ({xfails_file})")) xfail_id_matched[id_] = True break From 54c7141ddda5a7c36eda441353b715a9f7028548 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Tue, 18 Apr 2023 13:56:55 -0600 Subject: [PATCH 17/17] Fix a typo in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 38797dc3..fc60ac71 100644 --- a/README.md +++ b/README.md @@ -253,7 +253,7 @@ jobs: env: ARRAY_API_TESTS_MODULE: your.array.api.namespace run: | - export PYTHONPATH="${GITHUB_WORKSPACE}/array-api-compat" + export PYTHONPATH="${GITHUB_WORKSPACE}/your-array-library" cd ${GITHUB_WORKSPACE}/array-api-tests pytest -v -rxXfE --ci --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt array_api_tests/ ```