WIP: make tests runnable by pytest
#1013
Conversation
Otherwise, `pytest` tries collecting it because it starts with Test
Install the data files, and get their paths relative to the installed `grain` package
pyproject.toml
Outdated
There was a problem hiding this comment.
do we need to package the data?
i was hoping the we can eventually exclude the test files and the data from the wheel file
There was a problem hiding this comment.
In general, the package installed from a wheel knows nothing about the source: the wheel can in principle be installed on a different machine, for instance.
Are you thinking about two wheels, one for grain itself, and the other for grain-tests? Then the data files will only need to be included into the second one. But that would imply a larger reshuffling of test modules (for one, they'll need to move out of the main grain source tree).
There was a problem hiding this comment.
I was thinking more like run test files from the source against the installed grain version, but we can come back to this at some point later
There was a problem hiding this comment.
As far as I undertsand, this still requires some information about where to find these data files to get into the wheel. We can surely install a text file with a path to the source if that's what you're after. Feels a little unusual, TBH.
There was a problem hiding this comment.
Building with Bazel and running test with pytest causes this gap of information for pytest to where to find test-related data files.
In my opinion, for now we can package test-related files with the wheel, to run them with pytest - I think this change is simple enough that it doesn't cause much maintenance burden.
If in the future, the test suite data grows so large so that packing it with the wheel is unfeasible, then we could amend the build setup by moving test files manually in the build_whl.sh for testing phase - but this complication (which always needs to be maintained) to the build process makes sense when there's a valid need for it. WDYT?
| import grain | ||
| if grain.__file__ is None: | ||
| # tests run under bazel | ||
| self.testdata_dir = pathlib.Path(FLAGS.test_srcdir) / "testdata" |
There was a problem hiding this comment.
should we try setting this flag in the pytest scenario somehow?
There was a problem hiding this comment.
Okay, done now. Both ways of running tests seem to pass on my fork, ev-br#1
There was a problem hiding this comment.
pytest seems to have a fixture api through a conftest file that allows to provide initialization functions, e.g. in https://docs.pytest.org/en/stable/reference/reference.html#pytest.hookspec.pytest_sessionstart
the reason why I'm suggesting this is that the conftest.py file would be shared across all tests + would only be relevant to pytest automatically. As is, we'd have to update every test accessing local file system
There was a problem hiding this comment.
Currently the updated PR uses module-level fixtures in the modules which actually need this. We can make it into a session-level fixture, if you are not happy with module-level fixtures?
|
looks great, thanks! I'll work on importing this change internally |
Ported from #1013 PiperOrigin-RevId: 805035450
Ported from #1013 PiperOrigin-RevId: 805035450
Ported from #1013 PiperOrigin-RevId: 805035450
Ported from #1013 PiperOrigin-RevId: 805035450
This is WIP, get some 15 test failures locally.Updated according to an off-line discussion. CI tests pass with both pytest and bazel, at ev-br#1📚 Documentation preview 📚: https://google-grain--1013.org.readthedocs.build/