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

test_run should be a sh_test #76

Open
damienmg opened this issue Jun 13, 2016 · 11 comments
Open

test_run should be a sh_test #76

damienmg opened this issue Jun 13, 2016 · 11 comments

Comments

@damienmg
Copy link
Contributor

This would allow to test it with bazel test

To do so, shipping the source of the repository might be needed. See how it is done in bazel in src/test/shell/bazel/...

@ittaiz
Copy link
Member

ittaiz commented Jun 14, 2016

@johnynek seeing as test_run does bazel clean this means that running bazel test //... to get feedback on changes will take longer right?
Is it just something to live with since the repo is small? Is there anything to be done with it?

@damienmg
Copy link
Contributor Author

You want to recreate a very simple repo inside the sandbox for the test. This would clean only that repo. You have to depends on the bazel binary though.

@johnynek
Copy link
Member

here is the link: https://github.com/bazelbuild/bazel/tree/master/src/test/shell/bazel

this seems like a good thing to do for a shell-master that would like to contribute. Maybe we can reuse some of the above. Honestly, my shell programming is so weak, this would take me a lot of time, and I am not sure I'll have time to do it soon.

@or-shachar
Copy link
Contributor

or-shachar commented Feb 27, 2017

@damienmg @johnynek I'm gonna work on this task.

I'm evaluating different options for implementation and will be happy to hear your thoughts

Here are some of the challenges. Some of them were already mentioned above.

(A) to run test_run.sh we need the whole repo

like @damienmg , I had to allow copying the sources of each package and recreate the same workspace in the test run environment. I started with a very naive solution, will make it better in the future.

(B) running with bazel test uses vanilla env and causes long build time

just like @ittaiz mentioned. It takes a lot more time to do it inside bazel test run environment.
Running the bazel build alone took > 5 minutes on my machine (mac pro).
We have a theory that it's downloading scala binary on each run, which takes most of the time. I'm looking in the src/test/shell/bazel... for similar executions to find some optimization. If you can point me out to specific test target - would be awesome!

@damienmg - can you elaborate about your idea to recreate a very simple repo inside the sandbox? wouldn't it create a whole different test?

(C) splitting to different test targets

It seems like it would be clearer to run each run_test in a17 different test_sh targets. It would make the result more readable and allow parallel run in the future. Problem is that most tests rely on the bazel build step, and since bazel is creating isolated env for each test run - it would re-build all sources and re-import all dependencies for each step. What can we do about it? Should we leave it as one enormous test?

(D) Running bazel inside bazel execution behaves differently

As mentioned - I managed to recreate the workspace in the test environment, and run the test_run using sh_test
This is the output:

INFO: Found 1 test target...
running test bazel build test/...
Test bazel build test/... successful
running test bazel test test/...
Test bazel test test/... successful
running test bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges
Test bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges successful
running test bazel run test:JavaBinary
INFO: $TEST_TMPDIR defined: output root default is '/private/var/tmp/_bazel_ors/b6f1c770fafb6486701b6cea67bfc83b/execroot/rules_scala/_tmp/test_scala_1'. ____Loading complete. Analyzing... ____Found 1 target... Target //test:JavaBinary up-to-date: bazel-bin/test/JavaBinary.jar bazel-bin/test/JavaBinary ____Elapsed time: 0.146s, Critical Path: 0.00s ____Running command line: bazel-bin/test/JavaBinary /private/var/tmp/_bazel_ors/b6f1c770fafb6486701b6cea67bfc83b/execroot/rules_scala/_tmp/test_scala_1/_bazel_ors/9876f2205e80d7ca44b11b55acff9768/execroot/io_bazel_rules_scala/bazel-out/local-fastbuild/bin/test/JavaBinary: line 243: /private/var/tmp/_bazel_ors/b6f1c770fafb6486701b6cea67bfc83b/execroot/rules_scala/bazel-out/local-fastbuild/bin/test_scala.runfiles/local_jdk/bin/java: No such file or directory /private/var/tmp/_bazel_ors/b6f1c770fafb6486701b6cea67bfc83b/execroot/rules_scala/_tmp/test_scala_1/_bazel_ors/9876f2205e80d7ca44b11b55acff9768/execroot/io_bazel_rules_scala/bazel-out/local-fastbuild/bin/test/JavaBinary: line 243: exec: /private/var/tmp/_bazel_ors/b6f1c770fafb6486701b6cea67bfc83b/execroot/rules_scala/bazel-out/local-fastbuild/bin/test_scala.runfiles/local_jdk/bin/java: cannot execute: No such file or directory ERROR: Non-zero return code '126' from command: Process exited with status 126.

As you can see - first three steps are successful while the 4th step (run test:JavaBinary) prints a special INFO message and fails on No Such file.... Seems like the inner run inherits some properties from the outer bazel run and thus it's looking on wrong paths.
Any idea how to avoid it?

@johnynek
Copy link
Member

Thank you for working on this!

How are you building the inner workspace? Are you using the outer one as a local repository? I assume so. In the inner bazel, I think you could set the .bazelrc to use the worker mode, but now that I say that, I guess a comprehensive test would test in worker, standalone and sandbox modes.

As far as splitting up the run_test some of them are already bazel tests, only the failing tests (where the test is some other test or build fails) really need the nesting. Some others can just be totally normal tests, or sh_test in the main repo.

I don't have any ideas on issue 4 at the moment.

Thanks again!

@or-shachar
Copy link
Contributor

The following tests assume that bazel build / test already ran before them:

  • xmllint_test
  • text_repl
  • find -L ./bazel-testlogs -iname "*.xml"

When isolating the tests - they fail because there is no bazel run before them.

I currently removed them but how do you suggest to attack it? Add those lines to existing tests or add another bazel run before executing those tests?

@or-shachar
Copy link
Contributor

The solution seems to be dependent on the ability to run bazel inside sh_test, which doesn't work well ATM in some platforms (at least darwin and I haven't tested on windows).

Seems like there supposed to be another issue to enable doing that as simply as you can declare sources like @local_jdk//:jdk

@damienmg - Do you think I should open that issue in bazel repository or am I missing something here?

While I could explore options for solutions for this other issue, it seems like I'd need to get into the depths of bazel and not necessarily come up with something everyone can agree on.
I'll put this task aside until I get more mature with my bazel undertsanding, and perhaps in the meantime there would be a better way to run bazel in sh_test.

Thanks!

@damienmg
Copy link
Contributor Author

damienmg commented Mar 7, 2017

To run bazel inside sh_test you need to ship bazel and its depenendency (namely the JDK) to the test or it will fails with sandboxing.

You need to find some way to ship bazel (the jdk you can depends on @local_jdk//:jdk) inside your sh_test. I would suggest making a remote repository that get the bazel binary and put it in a remote repository.

@or-shachar
Copy link
Contributor

or-shachar commented Mar 7, 2017

I understand. That's what I mean - I'm couldn't find that way :-)
I tried couple of methods:

Defining "git_repostory"

I defined git_repository in the WORKSPACE "https://github.com/bazelbuild/bazel.git" but I had two problems with that:

  1. The repo is pretty huge, downloading it once took > 5 minutes.
  2. The targets there that create the binary , as well as their dependencies are package private.

"Stilling" the local bazel executable

Without diving into the executable internals, I tried to copy bazel executable (cp $(which bazel) bazel) but It failed to execute even without the sandboxing.

I created a dummy repository to test just that. I'll push those solutions to different branches so you can review it.

Thanks!

@damienmg
Copy link
Contributor Author

damienmg commented Mar 7, 2017

you can symlink it with an skylark remote repo, e.g.:

def _impl(rctx):
  bazel = rctx.os.which("bazel")
  ctx.symlink("bazel", bazel)
  ctx.file("BUILD.bazel", "filegroup(name = 'bazel-bin', srcs = ["bazel"])")

Of course this is an over simplification because bazel is probably not the bazel binary but the bazel wrapper...

@or-shachar
Copy link
Contributor

Thanks. Yes, it is a wrapper. I'll look into it after I'll feel more experienced with writing extensions. I'll keep you posted.

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

No branches or pull requests

4 participants