-
Notifications
You must be signed in to change notification settings - Fork 45
Add support for an xfails file, and allow the files to be specified with a flag #157
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
Changes from 14 commits
c6cb4fd
24f67f8
21d6b9f
f661284
153c9fa
373d05f
68470c6
8e2bff8
82cc470
234e9aa
d7b457a
44c7498
78a5b20
df7ffe3
7c8bbad
738efc4
fe878b0
1016a02
54c7141
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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,39 +209,81 @@ 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. | ||||||||||
|
||||||||||
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`. | ||||||||||
Comment on lines
+219
to
+228
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Much prefer highlighting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're just not going to get agreement on this. XFAILs are better than skips. Period. It's better to run the test so you can still see if it actually failed or not. The only exception is cases where the test hangs or crashes the testsuite. |
||||||||||
|
||||||||||
```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 <your array library> | ||||||||||
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 | ||||||||||
honno marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
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 --xfails-file ${GITHUB_WORKSPACE}/your-array-library/array-api-tests-xfails.txt array_api_tests/ | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this doesn't look quite right to me, but I might be missing something. The pythonpath/github_workspace stuff looks janky for a generic example, and we shouldn't be specifying Also, I much prefer highlighting skips rather than xfails.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "compat" thing is definitely a typo, but in general, this is a recipe for storing the skips in the array library repo and testing against the test suite. The point of GITHUB_WORKSPACE is to show how to do that (most people don't clone multiple repos in a single GitHub action, so it's not obvious how to do it). Again, I'd prefer to have a mostly working example so that people can just copy-paste it, rather than omitting stuff that they'll just have to figure out themselves. I always prefer it when people do this in other contexts which is why I've done it here. |
||||||||||
``` | ||||||||||
|
||||||||||
> **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 | ||||||||||
> 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 | ||||||||||
> 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. | ||||||||||
honno marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
|
||||||||||
## Contributing | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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] |
Uh oh!
There was an error while loading. Please reload this page.