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

[CCI migration] Disable travis tests except for RUN_FLAKE8 #747

Merged
merged 4 commits into from
Apr 29, 2020

Conversation

cpuhrsch
Copy link
Contributor

No description provided.

@cpuhrsch cpuhrsch requested a review from mthrok April 29, 2020 23:07
Copy link
Contributor

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Contributor

@zhangguanheng66 zhangguanheng66 left a comment

Choose a reason for hiding this comment

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

Why we want to disable the slow tests? Some datasets are covered only by slow tests.

@cpuhrsch
Copy link
Contributor Author

@zhangguanheng66 - This is only temporary. Moto is migrating the testing infrastructure over to circleci (which already runs much faster, albeit without slow tests).

Then once it's running on CCI we can enable their distributed caching for all the ~1GB Vector files and datasets and then hopefully get rid of "slow" tests in general.

For more details see this comment on the next steps.

@cpuhrsch cpuhrsch changed the title [CCI migration] Disable slow travis tests [CCI migration] Disable travis tests Apr 29, 2020
@cpuhrsch cpuhrsch changed the title [CCI migration] Disable travis tests [CCI migration] Disable travis tests except for RUN_FLAKE8 Apr 29, 2020
Christian Puhrsch added 2 commits April 29, 2020 16:20
@cpuhrsch cpuhrsch merged commit 53f2108 into pytorch:master Apr 29, 2020
@zhangguanheng66
Copy link
Contributor

We should get them covered by CCI ASAP.

@cpuhrsch
Copy link
Contributor Author

A lot of the slow tests already weren't working as you can see in this log associated to the PR that removes all slow tags as an example. The issue is that the slow tests pull in multiple GBs of data from the internet and the tests time out.

@zhangguanheng66
Copy link
Contributor

A lot of the slow tests already weren't working as you can see in this log associated to the PR that removes all slow tags as an example. The issue is that the slow tests pull in multiple GBs of data from the internet and the tests time out.

Typically, I expect those slow tests to finish with warnings because some of them take time to run (see bottom here). If the test fails, it means something we need to fix (correct me if I'm wrong).

@cpuhrsch
Copy link
Contributor Author

cpuhrsch commented Apr 30, 2020

@zhangguanheng66 - The only issue with these tests when they time out is that they, depending on the test, seem to never run at all. test_vectors_get_vecs for example seems to never run because it is taking so long. And even more so, if you remove the slow tags from the tests, it's the first one to fail whereas in the log you reference it doesn't show up anywhere except at the bottom together with the tests that all seem to have been skipped. Even though I'm a bit surprised it shows the amount of time it took to run it and otherwise doesn't mention the test.

But if this test never runs we should remove it or change it. I think if we cache the vectors it downloads the test might be able to run in a reasonable time and therefore will always run.

EDIT: When I run all tests (without slow tag) offline it takes about 20minutes the first time around and about 3minutes the second time, because it doesn't need to download the vectors. There is a total of 22GB of .vector_cache and 500MB of .data. If we can keep those around across jobs via cache (or remove the need for them), we can cut down significantly on the amount of time our jobs run to test. Of course there's value in exercising the download paths and such as well. We'll need to see what the balance here is or if there's a different way to test them.

@cpuhrsch cpuhrsch deleted the travis1 branch April 30, 2020 03:21
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.

None yet

3 participants