Be more robust in how regression data are discovered

[benjaminhwilliams]

Be stricter about where and how regression data are discovered, during testing. This fixes some tests that are failing during the Conda-forge release process.

As previously mentioned in #819 (comment):

• Ensure that, regardless of the test runner working directory, the regression data can be found.
• During zarr.tests.test_dim_separator.test_open, open an array in read-only mode, to prevent accidental creation of an empty array when the expected array does not exist, which would leave droppings in the test runner working directory.

TODO:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[benjaminhwilliams]

Release notes follow shortly.

[benjaminhwilliams]

@joshmoore, does this pass muster, as far as you're concerned? I'm not sure if I have correctly formatted the release notes.

[benjaminhwilliams]

As I'm a first-time contributor, the CI workflows won't run without maintainer approval. Would a kind maintainer mind enabling them?

[joshmoore]

Would a kind maintainer mind enabling them?

Done

I'm not sure if I have correctly formatted the release notes.

Looks great!

[codecov]

Codecov Report

Merging #821 (4a11512) into master (cba2783) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #821   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          31       31           
  Lines       10604    10606    +2     
=======================================
+ Hits        10598    10600    +2     
  Misses          6        6           

Impacted Files	Coverage Δ
zarr/tests/test_dim_separator.py	100.00% <100.00%> (ø)

[benjaminhwilliams]

I'm obviously unable to claim that all the CI actions have passed, nor that the project test coverage is 100%, but is this necessary here? The diff coverage is 100% and the missing project coverage is in an unrelated part of tests.test_dim_separator, so it doesn't seem appropriate to target that coverage in this PR.

[joshmoore]

is this necessary here?

Nope. This has been blocking my PRs as well. Don't worry. I've been focused on trying to fix conda and havedn't had time to fix codecov ;)

[benjaminhwilliams]

Don't worry. I've been focused on trying to fix conda and havedn't had time to fix codecov ;)

Good good. Thanks for the opportunity to tinker!

[benjaminhwilliams]

As an aside, off topic for this PR, I wonder if you could cheat Codecov by replacing the last three entries in the tuple of fixture parameters,

                        "fs_nested",
                        "fs_flat",
                        "fs_default"

with pytest.params,

needs_fsspec = pytest.mark.skipif(not have_fsspec, reason="needs fsspec")

...

                        pytest.param("fs_nested", marks=needs_fsspec),
                        pytest.param("fs_flat", marks=needs_fsspec),
                        pytest.param("fs_default", marks=needs_fsspec)

and then remove the if clause.

Of course, it doesn't actually change anything (Codecov would still be right to complain that the CI doesn't test in the absence of fsspec), but I believe it silences Codecov because there is no longer an explicit untested code path.

[joshmoore]

Thanks for the opportunity to tinker!

;) Glad to have other tinkerers.

A heads up that in an effort to not keep abusing releases in order to test conda-forge, I'm running locally with this diff to zarr-feedstock:

(base) /tmp/zarr-feedstock $git diff
diff --git a/recipe/meta.yaml b/recipe/meta.yaml
index a5c0beb..d60380e 100644
--- a/recipe/meta.yaml
+++ b/recipe/meta.yaml

 source:
-  fn: {{ name }}-{{ version }}.tar.gz
-  url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
-  sha256: {{ sha256 }}
+  git_url: https://github.com/benjaminhwilliams/zarr-python.git
+  git_rev: fix-fixture-finding

 build:
   number: 0
(base) /tmp/zarr-feedstock $./build-locally.py

Do you have any idea why I'm not seeing the failures locally?

[joshmoore]

I wonder if you could cheat Codecov by replacing the last three entries in the tuple of fixture parameters,

I imagine so. Happy to see that as well, which removes the need for my pragma in #822

[benjaminhwilliams]

Do you have any idea why I'm not seeing the failures locally?

What is your working directory when running Pytest? You will only see the failures on master when the working directory doesn't contain the sub-directory fixture, and this is true when the CI action runs in its own specially-created directory. Are you running Pytest in zarr-python?

[benjaminhwilliams]

I wonder if you could cheat Codecov by replacing the last three entries in the tuple of fixture parameters,

I imagine so. Happy to see that as well, which removes the need for my pragma in #822

Want me to add a PR for that too?

[joshmoore]

Want me to add a PR for that too?

If you'd like, sure!

[joshmoore]

NB: Merging in #822 has gotten this green. 👍

[joshmoore]

Are you running Pytest in zarr-python?

Yes.

[benjaminhwilliams]

Are you running Pytest in zarr-python?

Yes.

I think if you run Pytest from another directory, with something like

/tmp/some/dir/or/other$ pytest <wherever>/zarr-python/zarr

instead of

<wherever>/zarr-python$ pytest zarr

then you'll start to see the failures.

[joshmoore]

Thanks, @benjaminhwilliams. Looking forward to future PRs. 😉