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

tools: a number of targets are not cached and make bazel test //... slow #7437

Closed
stevekuznetsov opened this issue Mar 27, 2018 · 8 comments
Closed
Labels
area/bazel lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@stevekuznetsov
Copy link
Contributor

The majority of targets are cached without issue, but a couple (notably pylint) make bazel test //... really slow and annoying. Here's a run with one BAZEL.build file changed between this run and the last one:

$ bazel test //...
INFO: Analysed 2110 targets (538 packages loaded).
INFO: Found 1987 targets and 123 test targets...
INFO: Elapsed time: 87.446s, Critical Path: 68.61s
INFO: Build completed successfully, 121 total actions
//boskos:go_default_test                                        (cached) PASSED in 1.4s

...

//velodrome/transform/plugins:go_default_test                   (cached) PASSED in 1.2s
//hack:verify-pylint                                                     PASSED in 64.3s
//hack:verify-spelling                                                   PASSED in 14.6s
//hack:verify_boilerplate                                                PASSED in 3.8s

Executed 3 out of 123 tests: 123 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.

We should figure out how to make pylint and spelling and boilerplate incremental or faster.

/cc @ixdy @BenTheElder @rmmh

@BenTheElder
Copy link
Member

they python targets are not really cacheable because they take in the whole repo and aren't properly hermetic, we've got lints in a separate job while we decide what to do with them.

not sure about the velodrome / boskos tests.

@stevekuznetsov
Copy link
Contributor Author

Looks like pylint and spelling already parallelize, but boilerplate does not -- no super simple throughput wins that I can see. Maybe there is some bazel magic that can make pylint not run when no Python files have changed?

The velodrome and boskos got cached correctly, just didn't elide them from the output.

@ixdy
Copy link
Member

ixdy commented Mar 27, 2018

The verify tests will cache, but only if nothing in your repo has changed.

They're pretty un-bazely in that they depend on //:all-srcs, which depends on the entire tree; this isn't really a use-case for which bazel is designed. As a result, they have to be re-run anytime any file in the repo changes.

There was a proposed PR (kubernetes/repo-infra#46) to try to improve things slightly so we can have //:all-go-srcs and //:all-py-srcs etc, though this starts to get unwieldy pretty quickly.

In the past, we've tagged these as manual, so bazel test ... doesn't run them automatically. This has caused some complaints, since then the checks might fail in PR tests, since you didn't remember to run them locally.

(I personally think we shouldn't be running them with bazel test ...; maybe we can tag them as verify and then by default skip them, but allow a bazel test --config=verify which includes them, perhaps.)

@BenTheElder
Copy link
Member

BenTheElder commented Mar 27, 2018 via email

@stevekuznetsov
Copy link
Contributor Author

Yeah I guess I was hoping for one button that would do Go unit + Go lint + boilerplate + bazel verify, basically everything that I would generally hit if I were iterating on Prow, but not other targets. Maybe I'm just imagining a world where prow is in it's own repo :)

@rmmh
Copy link
Contributor

rmmh commented Mar 27, 2018

It's possible to do incremental/cached pylint results too, but probably not inside of bazel.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 25, 2018
@stevekuznetsov
Copy link
Contributor Author

No action items on this

/close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bazel lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

6 participants