WIP: issue-76 #156

or-shachar · 2017-03-01T13:55:53Z

working on #76
Instead of running tests using bash, it will use sh_tests.
To run this you can run
bazel test :all

Known issues that I need to find solution for:

All runs must share the same repository cache, and there must be one rule that will run before tests and download the libraries once (otherwise all tests would try to download the libraries in parallel to the same path). Currently you get consistent behavior and test time only from second run
The targets that run java fail because they cannot find java executable - I need your advise about how to make java available in sh_test env
I added .bashrc that define some rules for persistent worker and shared repository cache. Not sure how this will behave in CI, or if it's allowed.

@damienmg @johnynek - Of course this PR is mainly for your review and advise, and not for merge yet.

- moved to use bazel sh_test - separated each test to different target

bazel-io · 2017-03-01T13:55:54Z

Can one of the admins verify this patch?

googlebot · 2017-03-01T13:55:55Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

googlebot · 2017-03-01T14:12:02Z

CLAs look good, thanks!

…is configuration

damienmg

Sorry for taking so long, a bit of a rush here

damienmg · 2017-03-02T13:38:14Z

BUILD

+         "//tut_rule:sources",
+         "//twitter_scrooge:sources"]
+
+additions = [] 


What is additions for?

Bad naming... and currently redundant. At first I used this mechanism to only run tests that run bazel, the "additions" allowed me to define bazel arguments / flags that will be added to each execution. Later on I saw that shipping .bazelrc is much more effective because some other tests call bash function with inner calls to bazel.

damienmg · 2017-03-02T13:43:30Z

BUILD

+sh_test(
+    name = "build_targets_under_tests_package",
+    size = "large",
+    data = repo,


If you want the toolchain to be shiped in the test you need to pass "@local_jdk//:jdk", in the data dependency. You will also need bazel itself as a dependency and specify it to the --javabase option of bazel.

See https://github.com/bazelbuild/bazel/blob/master/src/test/shell/testenv.sh#L246
and https://github.com/bazelbuild/bazel/blob/master/src/test/shell/testenv.sh#L87

Adding the local_jdk://jdk was easy and we made some progress (It fails on another thing right now which is - it claims that it cannot load the main class JavaBinary... (look in travis).

As for "bazel itself as dependency with --javabase" - I tried to adjust Bazel's testenv.sh to our repository but it seems to have some dependencies on other packages that I couldn't find. I tried to simply add the --javabase <path> to the .bashrc but it did not make any different in the result...

So let me rephrase, you need:

to set --javabase and --host_javabase like it is done in testenv in Bazel

You will need to have "bazel" as a dependency to run inside the sandbox. Might not be important if it's on the PATH and installed outside of the workspace but I think it will fails on ci.bazel.io.

removed redundant "addition"

damienmg · 2017-03-02T17:02:16Z

Trigger bazel ci: test this please

or-shachar · 2017-03-02T17:06:02Z

@damienmg - I don't think that you run the test_sh script in Jenkins...
If I understand right the only thing you run there is bazel test test:...

damienmg · 2017-03-02T17:07:50Z

Right, we do bazel test //test/..., can you add a test_suite there that execute the sh_test?

or-shachar · 2017-03-02T17:09:08Z

Sure! But I think that once it's working it should be moved to a different location... like "intergration-tests" or something of a kind.

damienmg · 2017-03-02T17:10:13Z

sure it just implies another change to ci.bazel.io, so it's easier to do it that way right now.

or-shachar · 2017-03-02T17:23:15Z

Ok... I added another folder under test called integration-tests and put all tests there. You can trigger bazel ci: test again. Thanks

damienmg · 2017-03-02T17:47:17Z

test this please

ittaiz · 2017-03-02T19:34:51Z

.travis.yml


 script:
-  - bash test_run.sh
+  - bazel test test/integration-tests/...


why not bazel test test/...?

In previous solution it would only run the test_sh.sh which is the integration tests.
Inside the integration tests in runs the bazel test test/...

or-shachar · 2017-03-02T22:21:33Z

@damienmg you were right. Under platform darwin-x86_64 all tests fail because it cannot find bazel.

or-shachar · 2017-03-02T22:40:11Z

So we are left with several problems:

For some reason the run_java rules cannot load the main class. (with error message - Could not find or load main class *). Note that adding the --host_javabase... --javabase... to the inner bazel run did not make any difference on my local.
bazel is missing in some platforms and must be imported.
run_scalaBinary is possibly flaky (failed on timeout on Linux platform only on Jenkins).
test_disappearing_classes is failing because it tries to change the content of files in the workspace and when running inside bazel test - the file is in read only mode.

…on tests. Currently a call to "bazel" in an integration test means calling a (quite hidden) function in test-setup.sh which actually calls "$bazel" defined in "shell/bazel/testenv.sh" which is equal to "$(rlocation io_bazel/src/bazel)". This is extremely confusing and error prone. The new mechanism is to add a wrapper script to shell/bin called bazel and export this directory to the PATH. Moreover, not every test loads the same test environment, for instance consider how bazel_query_test loads the test environment: - Load shell/integration/testenv.sh which loads, - shell/bazel/test-setup.sh which loads, - shell/bazel/testenv.sh which loads, - shell/unittest.bash which loads, - shell/testenv.sh Again this is error prone and specially hard to understand, in fact each test writer needs to decide which of these testenv to load. This change fixes all of this by having only one testenv.sh and summarizing the test setup in integration_test_setup.sh. Namely, for any new integration test, the developer needs to load integration_test_setup to get the environment set up including the unittest framework (also it helps to attract contributions). This change also allows to open sourcing client_sigint_test: Since bazel was a function client_sigint_test was using a wrong process id to interrupt the build. The problem is that $! returns bash's id instead of the id of the process running in the background when using a function instead of an executable. A few tests needed to be adapted to the new infrastructure. -- MOS_MIGRATED_REVID=136470360

ittaiz · 2017-05-30T08:59:15Z

just had a talk with @dslomov which proposed using sh_binary on the downloaded http_archive and having the sh_test depend on the sh_binary target to get it into the sandbox.
WDYT?

bazel-io · 2017-05-30T08:59:16Z

Can one of the admins verify this patch?

ittaiz · 2018-01-18T09:11:43Z

obviously we won't resurrect this. If we'll want concepts we'll take a look at the branch.

issue-76 WIP (failing on java run):

b05abd6

- moved to use bazel sh_test - separated each test to different target

googlebot added the cla: no label Mar 1, 2017

modified travis build config

82672fc

googlebot added cla: yes and removed cla: no labels Mar 1, 2017

or-shachar added 9 commits March 1, 2017 16:23

removed .bazelrc file

faf29ce

add test configuration to print log on errors

6f6be1e

removed redundant ls

a7e5f51

added .bazelrc to each test env so inner bazel run would inherit trav…

d771137

…is configuration

upgraded bazel version and added back default .bashrc

63df729

Added missing comma

0d3d830

removed run of xmllint_test as it's not stateless

2a2b97b

Fixed test_repl to not use git mechanism

a83eb66

removed stateful test test_repl

39d3594

damienmg reviewed Mar 2, 2017

View reviewed changes

added local_jdk to test that run java

2fb417d

removed redundant "addition"

moved integration tests inside test

7367c73

ittaiz reviewed Mar 2, 2017

View reviewed changes

ittaiz closed this Jan 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: issue-76 #156

WIP: issue-76 #156

or-shachar commented Mar 1, 2017 •

edited

Loading

bazel-io commented Mar 1, 2017

googlebot commented Mar 1, 2017

googlebot commented Mar 1, 2017

damienmg left a comment

damienmg Mar 2, 2017

or-shachar Mar 2, 2017

damienmg Mar 2, 2017

or-shachar Mar 2, 2017

damienmg Mar 2, 2017

damienmg commented Mar 2, 2017

or-shachar commented Mar 2, 2017 •

edited

Loading

damienmg commented Mar 2, 2017

or-shachar commented Mar 2, 2017

damienmg commented Mar 2, 2017

or-shachar commented Mar 2, 2017

damienmg commented Mar 2, 2017

ittaiz Mar 2, 2017

or-shachar Mar 2, 2017

or-shachar commented Mar 2, 2017

or-shachar commented Mar 2, 2017

ittaiz commented May 30, 2017

bazel-io commented May 30, 2017

ittaiz commented Jan 18, 2018

WIP: issue-76 #156

WIP: issue-76 #156

Conversation

or-shachar commented Mar 1, 2017 • edited Loading

bazel-io commented Mar 1, 2017

googlebot commented Mar 1, 2017

googlebot commented Mar 1, 2017

damienmg left a comment

Choose a reason for hiding this comment

damienmg Mar 2, 2017

Choose a reason for hiding this comment

or-shachar Mar 2, 2017

Choose a reason for hiding this comment

damienmg Mar 2, 2017

Choose a reason for hiding this comment

or-shachar Mar 2, 2017

Choose a reason for hiding this comment

damienmg Mar 2, 2017

Choose a reason for hiding this comment

damienmg commented Mar 2, 2017

or-shachar commented Mar 2, 2017 • edited Loading

damienmg commented Mar 2, 2017

or-shachar commented Mar 2, 2017

damienmg commented Mar 2, 2017

or-shachar commented Mar 2, 2017

damienmg commented Mar 2, 2017

ittaiz Mar 2, 2017

Choose a reason for hiding this comment

or-shachar Mar 2, 2017

Choose a reason for hiding this comment

or-shachar commented Mar 2, 2017

or-shachar commented Mar 2, 2017

ittaiz commented May 30, 2017

bazel-io commented May 30, 2017

ittaiz commented Jan 18, 2018

or-shachar commented Mar 1, 2017 •

edited

Loading

or-shachar commented Mar 2, 2017 •

edited

Loading