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

karma_web_test should create junit reporting outputs #983

Closed
josephperrott opened this issue Aug 8, 2019 · 15 comments
Closed

karma_web_test should create junit reporting outputs #983

josephperrott opened this issue Aug 8, 2019 · 15 comments
Labels
Can Close? We will close this in 30 days if there is no further activity enhancement

Comments

@josephperrott
Copy link
Collaborator

🚀 feature request

Relevant Rules

karma_web_test

Description

karma_web_test should provide junit reporting output to be consumed by tooling.

Describe the solution you'd like

karma_web_test should automatically create junit reporting outputs.

Describe alternatives you've considered

I would expect that it is possible to provide a karma config which triggers junit reporting, it seems that it makes the most sense to have the reporting automatically created.

@alexeagle
Copy link
Collaborator

I think @jvanallen93 is going to take a stab at this

@joshvanallen
Copy link
Contributor

joshvanallen commented Sep 21, 2019

I think I might be overthinking this, this seems pretty straight forward and just setting the outputFile property to XML_OUTPUT_FILE.
I'm curious though, how would we want to test this, if it needs to be tested at all?

@alexeagle
Copy link
Collaborator

@jvanallen93 yes it should have some testing.

We want to assert that you run bazel test and afterwards you get something in the dist/testlogs folder (note the test.xml file is already there for some of our tests e.g. dist/testlogs/internal/npm_install/compile_generate_build_file_check_compiled/test.xml)

That means we need a test that runs Bazel, so there are two options:

  • add an assertion around one of the existing integration tests in /examples or /e2e - these already run Bazel-in-bazel. examples/web_testing looks like it already does some karma-*-reporter stuff. So I would expect that bazel test //examples:examples_web_testing would do the things it does today and then also assert on what's in the test.xml output file. I think internal/bazel_integration_test/test_runner.js is the spot where we've just finished running Bazel and have a chance to look around the disk before exiting zero or one.
  • OR you could possibly do something cheap and edit one of the CI configurations to make an additional assertion after running bazel. This is harder to maintain though since only the CI would know how to run this test; it would be hard to reproduce any failures locally

@joshvanallen
Copy link
Contributor

Thanks for the insight and feedback @alexeagle!

@joeljeske
Copy link
Contributor

Is there any progress on this?

Could this solution be reused for the "jasmine_test" rule as well?

@Toxicable
Copy link

This here: https://github.com/bazelbuild/rules_nodejs/pulls/Toxicable
This provides a proper integration with bazel coverage. What it currently does is takes in v8 coverage .json files produced by the v8 runtime and converts them into lcov which bazel knows how to make combined reports with.

We could extend it to take in whatever format karma can give us, convert it to lcov, let bazel process it.
Once you have a combined report from bazel then you can run whatever tool on that to go to some other format.

Happy to take PR's on this extending from my curren work

@joeljeske
Copy link
Contributor

Sorry, it thought this ticket was for junior test reports, not coverage per se. Or does your PR handle writing junit reports to show the tests run?

@Toxicable
Copy link

Ah right, my mistake I misunderstood junit reporting.
I was thinking it was a format for coverage reporting not your test runner reporting.
With that in mind I don't think that PR will help at all with this issue :P

@joshvanallen
Copy link
Contributor

Yeah sorry peepz....life got busy. I believe I had it working, at least for Karma, just needed to spend time getting the tests in place. Now I think I'm getting some free time that I can pick this up again.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Oct 15, 2020
@KrauseStefan
Copy link
Contributor

This is still an issue that would be nice to have solved, I don't think it should be closed...
@jvanallen93 are you still working on this?

@alexeagle alexeagle removed the Can Close? We will close this in 30 days if there is no further activity label Oct 15, 2020
@sgammon
Copy link
Contributor

sgammon commented Dec 14, 2020

AFACT this has been solved for some time now, at least when i use karma_web_test locally it produces a test.xml under <out>/testlogs.

@KrauseStefan
Copy link
Contributor

After trying this out again it seems like it is now possible to use the karma-junit reporter plugin without patching the karma rules.
There is however very little documentation on how.

The trick is to add the plugin to the list of peer_deps of the karma rule, remember to include the defaults in the list as well.
The karma Junit reporter must be configured in the karma config file as documented by the plugin.
The plugin must also be configured to save the report to a file defined by the bazel environment variable XML_OUTPUT_FILE.

I don't know when the karma rule was changed to allow adding custom reporters but it works now.

This was tested with latest version of rule_nodejs and https://github.com/karma-runner/karma-junit-reporter#readme

I think this issue should be kept open with the intention of documenting how to add the junit reporter.
I also think an examples should be updated to use the junit reporter this will also ensure that this keeps working.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Mar 15, 2021
@github-actions
Copy link

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? We will close this in 30 days if there is no further activity enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants