Skip to content
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

Fast failing test in ci #1666

Merged
merged 13 commits into from
Apr 17, 2020
Merged

Fast failing test in ci #1666

merged 13 commits into from
Apr 17, 2020

Conversation

gabrieldemarmiesse
Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse commented Apr 14, 2020

Related to #1655

So of course this doesn't make our CI any faster, but it provides fast feedback for contributors. With this new test, we catch most of the errors quickly since many changes are related to python-only code. The test runs in 4m and 20s, much faster than the 11m and 25s needed to run our fastest test (macos).

Some ressources:

@gabrieldemarmiesse gabrieldemarmiesse marked this pull request as ready for review April 14, 2020 11:34
with:
python-version: 3.5
- uses: actions/checkout@v2
- run: pip install -r tools/install_deps/tensorflow-cpu.txt -r tools/install_deps/pytest.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried, but I removed it in 0083239 because we didn't get any speed benefits.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried another build After the cache build?

Copy link
Member Author

@gabrieldemarmiesse gabrieldemarmiesse Apr 15, 2020

Choose a reason for hiding this comment

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

Yes, you can take a look for yourself since the build logs are public for each commit. If I missed something, please let me know.

Copy link
Member

@seanpmorgan seanpmorgan left a comment

Choose a reason for hiding this comment

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

LGTM.. pytest in the resource_loader is not ideal IMO, but there's significant speed up and import looks well guarded

@seanpmorgan seanpmorgan merged commit bdb9a00 into tensorflow:master Apr 17, 2020
jrruijli pushed a commit to jrruijli/addons that referenced this pull request Dec 23, 2020
* Added a pytest option to skip the loading of custom ops.

* Added a ci run for python-only tests.

* Fix the usage of load_op_library.

* Revert "Fix the usage of load_op_library."

This reverts commit 94db295.

* Fixture is executed after collection.

* Removed assert False.

* Try running on macos for speed.

* Fix path.

* Run on ubuntu directly.

* Add caching for pip.

* Indent.

* Better wording.

* Remove the cache because we don't get any benefits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants