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

Arcanist Integration #145

Closed
schroederc opened this issue Apr 20, 2015 · 22 comments
Closed

Arcanist Integration #145

schroederc opened this issue Apr 20, 2015 · 22 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request

Comments

@schroederc
Copy link
Contributor

It'd be nice to have support for Bazel as a ArcanistUnitTestEngine so that arc unit and arc diff run all of a workspace's tests before submitting a review to Phabricator.

Kythe current has primitive arcanist support for its Bazel-like build system. It shouldn't be too hard for me to port it specifically for Kythe, but perhaps it would make a good contribution to the Bazel repo for other projects. Thoughts?

@ulfjack
Copy link
Contributor

ulfjack commented Apr 21, 2015

Not having worked with Phabricator or Arcanist, I don't follow what the
proposal is. Can you point us at relevant documentation? I looked for
ArcanistUnitTestEngine, which doesn't seem to apply, and is not really
documented.

On Tue, Apr 21, 2015 at 12:26 AM, Cody Schroeder notifications@github.com
wrote:

It'd be nice to have support for Bazel as a ArcanistUnitTestEngine so that arc
unit and arc diff run all of a workspace's tests before submitting a
review to Phabricator.

Kythe https://github.com/google/kythe current has primitive arcanist
support for its Bazel-like build system. It shouldn't be too hard for me to
port it specifically for Kythe, but perhaps it would make a good
contribution to the Bazel repo for other projects. Thoughts?


Reply to this email directly or view it on GitHub
#145.

@laszlocsomor laszlocsomor added type: feature request question P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Apr 21, 2015
@hanwen
Copy link
Contributor

hanwen commented Apr 21, 2015

I'm trying to get a bazel-examples repo out there, which would be a good place for contributions like these.

@schroederc
Copy link
Contributor Author

arc supports running a set of unit tests based on the files within a particular change (or --everything). It accomplishes this through a plugin system where each unit test system must write a PHP class that will execute the tests and parse the results into an array of ArcanistUnitTestResults.

See CampfireTestEngine.php for Kythe's current support.

@ulfjack
Copy link
Contributor

ulfjack commented Apr 21, 2015

That's cool. Yes, I guess we could hook that up with blaze query. For the
test results, we're planning to output the 'junit' xml format (which is
really ant xml) from the test runners.

On Tue, Apr 21, 2015 at 6:20 PM, Cody Schroeder notifications@github.com
wrote:

arc supports running a set of unit tests based on the files within a
particular change (or --everything). It accomplishes this through a
plugin system where each unit test system must write a PHP class that will
execute the tests and parse the results into an array of
ArcanistUnitTestResults.

See CampfireTestEngine.php
https://github.com/google/kythe/blob/master/buildtools/arc/unit/engine/CampfireTestEngine.php
for Kythe's current support.


Reply to this email directly or view it on GitHub
#145 (comment).

@schroederc
Copy link
Contributor Author

Is there any current way to output a structured format of the test results?

@hanwen
Copy link
Contributor

hanwen commented May 1, 2015

we write a .cache_status file which is a protobuf. It's not been intended as a public API, but the analogon inside google is used to collect test/build status results.

@schroederc
Copy link
Contributor Author

I also found that it is hard to query a file's rdeps when a file may or may not be covered by a build target. Bazel will just fail if it is given some random file. Is there a way to ignore such files during a query?

@ulfjack
Copy link
Contributor

ulfjack commented May 4, 2015

How does it fail?

On Fri, May 1, 2015 at 11:39 PM, Cody Schroeder notifications@github.com
wrote:

I also found that it is hard to query a file's rdeps when a file may or
may not be covered by a build target. Bazel will just fail if it is given
some random file. Is there a way to ignore such files during a query?


Reply to this email directly or view it on GitHub
#145 (comment).

@schroederc
Copy link
Contributor Author

If you query a file path that is used in a target, it prints the path as a target of the same package:

$ bazel query kythe/go/storage/tools/write_entries/write_entries.go
//kythe/go/storage/tools:write_entries/write_entries.go

Querying a file that is not declared ends up with this kind of error:

bazel query kythe/go/storage/tools/CAMPFIRE
ERROR: no such target '//kythe/go/storage/tools:CAMPFIRE': target 'CAMPFIRE' not declared in package 'kythe/go/storage/tools'; however, a source file of this name exists.  (Perhaps add 'exports_files(["CAMPFIRE"])' to kythe/go/storage/tools/BUILD?) defined by /usr/local/google/home/schroederc/kythe/kythe/go/storage/tools/BUILD.

This makes it hard to do this general query:

files=($(git ls-files -m))
bazel query "rdeps(//..., set(${files[@]}))"

The above query also fails when given files contains a path at the root of the WORKSPACE (no dirname).

@ulfjack
Copy link
Contributor

ulfjack commented May 4, 2015

Does -k help?

On Monday, May 4, 2015, Cody Schroeder notifications@github.com wrote:

If you query a file path that is used in a target, it prints the path as a
target of the same package:

$ bazel query kythe/go/storage/tools/write_entries/write_entries.go
//kythe/go/storage/tools:write_entries/write_entries.go

Querying a file that is not declared ends up with this kind of error:

bazel query kythe/go/storage/tools/CAMPFIRE
ERROR: no such target '//kythe/go/storage/tools:CAMPFIRE': target 'CAMPFIRE' not declared in package 'kythe/go/storage/tools'; however, a source file of this name exists. (Perhaps add 'exports_files(["CAMPFIRE"])' to kythe/go/storage/tools/BUILD?) defined by /usr/local/google/home/schroederc/kythe/kythe/go/storage/tools/BUILD.

This makes it hard to do this general query:

files=($(git ls-files -m))
bazel query "rdeps(//..., set(${files[@]}))"

The above query also fails when given files contains a path at the root
of the WORKSPACE (no dirname).


Reply to this email directly or view it on GitHub
#145 (comment).

@schroederc
Copy link
Contributor Author

Yeah, -k helps the query. Thanks

@perezd
Copy link
Contributor

perezd commented Jan 16, 2016

Any updates on this? I'd love for this to be supported natively.

@damienmg
Copy link
Contributor

The Java test runner has landed in 0e396b8 and Philip has a inflight change for making sandboxing allow to output XML file. It should arrive somewhere next week. For now you can have the xml outputs by disabling sandboxing.

For the rest of the piping to Arcanist, we would welcome any contribution :)

@perezd
Copy link
Contributor

perezd commented Jan 16, 2016

OK, I can give it a shot. Any docs on how to utilize it?

@damienmg
Copy link
Contributor

I don't really know what kind of doc you are expecting but if you run bazel
with --spawn_strategy=standalone --nolegacy_bazel_java_test you should
get XML output for every java test available in bazel-testlogs.

On Sat, Jan 16, 2016, 7:34 PM Derek Perez notifications@github.com wrote:

OK, I can give it a shot. Any docs on how to utilize it?


Reply to this email directly or view it on GitHub
#145 (comment).

@perezd
Copy link
Contributor

perezd commented Jan 16, 2016

Oh, this hasn't hit a release version yet has it?

@damienmg
Copy link
Contributor

No it should be in 0.2 (somewhere at the end of the month).

On Sat, Jan 16, 2016 at 7:51 PM Derek Perez notifications@github.com
wrote:

Oh, this hasn't hit a release version yet has it?


Reply to this email directly or view it on GitHub
#145 (comment).

@damienmg
Copy link
Contributor

(and even then it should still requires the --nolegacy_bazel_java_test flag)

On Sat, Jan 16, 2016 at 7:52 PM Damien Martin-guillerez dmarting@google.com
wrote:

No it should be in 0.2 (somewhere at the end of the month).

On Sat, Jan 16, 2016 at 7:51 PM Derek Perez notifications@github.com
wrote:

Oh, this hasn't hit a release version yet has it?


Reply to this email directly or view it on GitHub
#145 (comment).

@perezd
Copy link
Contributor

perezd commented Jan 16, 2016

Cool, please ping this thread when its in a release and I'll make it happen.

@kchodorow
Copy link
Contributor

It is now in a release.

@damienmg
Copy link
Contributor

I am closing this one as we now have XML output for every test and query. There is no more feature that should be needed on bazel side.

@igorgatis
Copy link

I managed to make a more lightweight version of Kythe's bazel-arcanist integration. It uses --build_event_json_file and it is (IMHO) more developer friendly because it shows progress using bazel's output: https://github.com/igorgatis/bazel-arcanist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants