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

More comprehensive JaCoCo support #1120

Merged
merged 8 commits into from
Dec 12, 2018
Merged

Conversation

Thilas
Copy link
Contributor

@Thilas Thilas commented Oct 13, 2018

Following work done on #920, here are some changes that bring a more comprehensive support of JaCoCo format.

Basically, it now fully supports JaCoCo format (updated to 1.1):

  • add class elements for each script file
  • add method elements for each method in a script file (as well as a script() method for code with no method)
  • use sourcefilename attributes to link coverage with source code
  • add counter elements wherever it is possible to

All this allows to generate cool reports like the ones created with ReportGenerator thanks to the (very) recent support of JaCoCo.

Also, to minimize changes with current Pester's behavior, I removed the -DetailedCodeCoverage parameter added in #920 and the current coverage report is back in the console.

And of course, unit tests have been adjusted to check the new xml generated.

Let me know if you have any question. Thanks!

@Thilas
Copy link
Contributor Author

Thilas commented Oct 14, 2018

Any idea why "Publish Status to GitHub (Pester)" check has failed?

@nohwnd
Copy link
Member

nohwnd commented Oct 16, 2018

@Thilas the error says: "The process cannot access the file
[02:19:20]'C:\TeamCity\BuildAgent\work\93ac3fcead2a62ac\Test.v2.xml' because it is being
[02:19:20]used by another process.", but I am not sure what exactly happened there. I've run the build again to see if it was just a one-time issue.

@Thilas
Copy link
Contributor Author

Thilas commented Oct 16, 2018

@nohwnd, thank you for the feedback. I've just figured out how to get the error myself (logging in as a guest). What's even stranger here is that it fails only on PowerShell v2, version on which code coverage doesn't apply (and I didn't change a thing on this part).

I don't see any new build here. I guess you didn't run a new one yet?

Otherwise, any thought about the PR?

@nohwnd nohwnd self-assigned this Oct 19, 2018
@Thilas
Copy link
Contributor Author

Thilas commented Nov 8, 2018

@nohwnd, any news about this?

@nohwnd
Copy link
Member

nohwnd commented Nov 8, 2018

@Thilas the tests keep failing with the same error, it's likely a problem with the build server, because it's not only this build that fails. Unfortunately I have no power over the build server. @dlwyatt could you help please? It's the same problem as before. A file is locked and makes the tests fail.

@Thilas
Copy link
Contributor Author

Thilas commented Nov 25, 2018

@dlwyatt, any news about this? Thanks.

@Thilas
Copy link
Contributor Author

Thilas commented Nov 30, 2018

@nohwnd, tests are now passing. What's next? Some reviews I guess? Thanks for your help.

@nohwnd
Copy link
Member

nohwnd commented Dec 4, 2018

@Thilas I will review it and let you know :) Thanks for you patience.

@nohwnd
Copy link
Member

nohwnd commented Dec 12, 2018

@Thilas I merged #920 into master, and master into your changes, if it builds I will merge it in master. There were few merge conflicts but it should be all good now. Please give it a quick look if you have time.

Also could you share on which build server you use this? And possibly test it there again before I merge? It definitely has to work on Azure pipelines.

@nohwnd nohwnd merged commit ec44a1f into pester:master Dec 12, 2018
@nohwnd
Copy link
Member

nohwnd commented Dec 12, 2018

Merged thanks!

@Thilas
Copy link
Contributor Author

Thilas commented Dec 12, 2018

Thanks @nohwnd. You already merged the branch, but I'll try this anyway to provide the feebacks you wanted.

@nohwnd
Copy link
Member

nohwnd commented Dec 13, 2018

@Thilas yes that would be great. I plan to release in few days. Thank you.

@Thilas
Copy link
Contributor Author

Thilas commented Dec 13, 2018

My tests are ok. Let's release it!

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

Successfully merging this pull request may close these issues.

4 participants