Skip to content

[ENH] test suite cleanup #950

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mtsokol
Copy link
Contributor

@mtsokol mtsokol commented Jul 18, 2025

Hi @iindyk,

This PR fixes ubuntu-arm JAX import failures by moving these test to after bazel build/test.
It also adds skips to all tests that are incompatible with Windows platform.

With this change "Run unit tests ✅" should work, here's a job: https://github.com/mtsokol/grain/actions/runs/16372765054


📚 Documentation preview 📚: https://google-grain--950.org.readthedocs.build/

@mtsokol mtsokol force-pushed the test-suite-cleanup branch 2 times, most recently from 97527d9 to 0ab7035 Compare July 21, 2025 07:47
@mtsokol mtsokol force-pushed the test-suite-cleanup branch from 0ab7035 to bc85005 Compare July 21, 2025 07:49
Copy link
Collaborator

@iindyk iindyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@@ -126,6 +126,16 @@ main() {
$PYTHON_BIN -m pip install --find-links="${OUTPUT_DIR}/all_dist" --pre "${PKG_NAME}"
$PYTHON_BIN -m pip install jax
$PYTHON_BIN grain/_src/core/smoke_test_with_jax.py
$PYTHON_BIN grain/_src/core/tree_lib_jax_test.py
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having few tests run differently from the rest is a bit brittle imo. Is there a way for us to run a test suite with something like pytest? or unittest discover?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right here is just a minimal change to make it work - let me move that to actual pytest call tomorrow!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, make sense, thanks. I think ideally we would run the whole test suit together in the long term

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants