Add a subsuite of unit tests for ML package and remove them from 'coverage' GHA workflow. #31255
Conversation
tvalentyn
changed the title
tvalentyn
changed the title
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
| on: | ||
| pull_request_target: | ||
| branches: [ "master", "release-*" ] | ||
| paths: [ "model/**","sdks/python/**","release/**", 'release/trigger_all_tests.json', '.github/trigger_files/beam_PreCommit_Python_ML.json'] |
There was a problem hiding this comment.
No-op right now, but we may eventually consider making this only trigger on ml changes
There was a problem hiding this comment.
Also seen anecdotal evidence of using pytest-testmon, https://stackoverflow.com/questions/67398655/run-only-tests-which-depend-on-the-change.
we could give it an have a full postcommit coverage as backup
| setenv = | ||
| extras = test,gcp,dataframe,ml_test | ||
| commands = | ||
| bash {toxinidir}/scripts/run_pytest.sh {envname} "{posargs}" |
There was a problem hiding this comment.
Should this even accept posargs? Won't it always be apache_beam/ml?
There was a problem hiding this comment.
yeah, we can configure the path here instead of in the GHA yaml. it would be less consistent with other targeted GHA suites but still straightforward.
There was a problem hiding this comment.
i took another look, i'd rather keep it as is to make uniform configuration across test suites.
| gradle-command: :sdks:python:test-suites:tox:py${{steps.set_py_ver_clean.outputs.py_ver_clean}}:testPy${{steps.set_py_ver_clean.outputs.py_ver_clean}}ML | ||
| arguments: | | ||
| -Pposargs=apache_beam/ml/ \ | ||
| -PpythonVersion=${{ matrix.python_version }} |
There was a problem hiding this comment.
Noted elsewhere, but it is odd that we have to specify this for testPy311ML - I'd assume that's implied by the task name
There was a problem hiding this comment.
yes, that shouldn't be necessary.
There was a problem hiding this comment.
looks like this was necessary to make sure the virtual environment that creates an sdist is created using a python version that is installed on GHA workers.
There was a problem hiding this comment.
revert https://github.com/apache/beam/pull/31283/files should help
…nfig already defines the Python version.
|
Assigning reviewers. If you would like to opt out of this review, comment R: @damccorm for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Add a subsuite of unit tests for ML package and run it with all known optional ML dependencies.
The suite should provide early detection signals when an ML dependency creates a new release, and we are no longer compatible with its API, but it could also create noise for us. Given that the ML dependencies are optional for Beam, we don't set version bounds for these test dependencies. We install a lot of (direct+transitive) dependencies in the
ml_testextra , and leave dependency resolution up to pip. If dependency resolution becomes messy, we will have the option to tweak the dependencies or introduce more specialized extras.Compatibility testing between Beam with specific fine-grained versions of particular dependencies is done in a separate postcommit suite .github/workflows/beam_PostCommit_Python_Dependency.yml.
Moving compat tests from 'coverage' suite reduces the time run the code coverage GHA. Tests that need extra ML deps were not included in code coverage tests previously, keeping it that way for now to reduce coverage runtime, but we can reconsider if we find this signal valuable.
I exclude the
apache_beam/mltests from the main precommit suite, since the new suite includes the same tests + some unit tests that were skipped when optional dependencies are not installed. If this test suite becomes too unstable, we might have to addapache_beam/mlwithout extra deps back to the precommit suite.Adding this suite exposed several tests that were not running and not working properly: #31254, #31252, #31287, #31294 and reduced the 'coverage' suite run to under 30 min: https://github.com/apache/beam/actions/runs/9054795242/job/24875055178?pr=31255
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.