Add code coverage report generation instructions by rjmholt · Pull Request #6515 · PowerShell/PowerShell

rjmholt · 2018-03-27T21:30:50Z

PR Summary

Adds a document to the repo that details how to obtain code coverage analysis for PowerShell using OpenCover.

Would be happy with any feedback on:

Extra steps or info that should be included
Using PowerShell highlighting in code blocks, rather than no highlighting
Whether to include a prompt or not
Chopping down parameters with newlines (was thinking of smaller laptop screens)

We should also update the code coverage blog posts to possibly point at this document.

PR Checklist

SteveL-MSFT · 2018-03-27T22:34:52Z

@rjmholt FYI - the assignees should only be maintainers. You can add people to reviewers.

rjmholt · 2018-03-27T22:48:16Z

@SteveL-MSFT Understood

SteveL-MSFT · 2018-03-27T23:12:40Z

docs/testing-guidelines/getting-code-coverage.md

@@ -0,0 +1,127 @@
+Getting Code Coverage Analysis for PowerShell
+===


I believe we've standardized on using hashes for headers

I'll change it over, although my opinion is that the hashes are less readable in the raw view.

SteveL-MSFT · 2018-03-27T23:13:08Z

docs/testing-guidelines/getting-code-coverage.md

+
+**Note: Code coverage is currently only supported on Windows, since we use OpenCover**
+
+The PowerShell code base is configured to build with code coverage support using [OpenCover](https://github.com/OpenCover/opencover). You can see the testing coverage of the current [`master`](https://github.com/PowerShell/PowerShell) branch build at any time at [codecov.io](https://codecov.io/gh/PowerShell/PowerShell).


Put the second sentence on a separate line (semantic linebreak)

SteveL-MSFT · 2018-03-27T23:13:54Z

docs/testing-guidelines/getting-code-coverage.md

+The PowerShell code base is configured to build with code coverage support using [OpenCover](https://github.com/OpenCover/opencover). You can see the testing coverage of the current [`master`](https://github.com/PowerShell/PowerShell) branch build at any time at [codecov.io](https://codecov.io/gh/PowerShell/PowerShell).
+
+To run test coverage analysis of PowerShell on your own branch/machine
+however, you will need to take the following steps (and be aware that running


"however," seems out of place and unnecessary

SteveL-MSFT · 2018-03-27T23:14:22Z

docs/testing-guidelines/getting-code-coverage.md

+Running tests with code coverage analysis
+---
+
+**First**: Open PowerShell in an **Administrator** session. OpenCover


replace Administrator with elevated

SteveL-MSFT · 2018-03-27T23:15:47Z

docs/testing-guidelines/getting-code-coverage.md

+PS> Import-Module $PWD\test\tools\OpenCover
+
+# Install OpenCover to a temporary directory
+PS> Install-OpenCover -TargetDirectory $env:TEMP -Force


I thought we updated this cmdlet so it has a default TargetDirectory so it doesn't need to be specified?

The original blog post gives an argument (except with no parameter, and Install-OpenCover's first positional argument is actually the URL from which to download OpenCover). I'm happy with using a default, but in that case I think we should write some documentation of build function parameters so that Get-Help has some description of what the parameters do.

Looking at the file, the current default TargetDirectory is $HOME

If the defaults work, I would recommend not adding additional parameters as it makes it more confusing to new users

SteveL-MSFT · 2018-03-27T23:16:13Z

docs/testing-guidelines/getting-code-coverage.md

+# If you want to run only the continuous integration tests,
+# add -CIOnly, which will take less time
+PS> Invoke-OpenCover `
+>> -OutputLog coverage.xml `


I believe we also have reasonable defaults for these

If we use the default OpenCoverPath in Install-OpenCover, we should do that here too.

I assume the function is designed to be run in a specific place, and in that case has sane defaults for TestPath and PowerShellExeDirectory (in which case we can take out the line defining $psDir).

But in the case of OutputLog I think we should keep that so anyone following along knows where the output will be.

In general, I'm sort of in favour of using explicit parameters in the documentation, only because there's more information to work with if something goes wrong. It's why PowerShell has named parameters after all.

I say all this after having to read the scripts to work out what the parameters do

The defaults here are:

OutputLog: "$home/Documents/OpenCover.xml"

TestPath: "${script:psRepoPath}/test/powershell"

OpenCoverPath: "$home/OpenCover"

PowerShellExePath: "${script:psRepoPath}/src/powershell-win-core/bin/CodeCoverage/netcoreapp2.0/win7-x64/publish"

So I feel like TestPath and PowerShellExePath are pretty sane and we can omit them happily.

For OpenCoverPath we should only use the default path if Install-OpenCover does (I'm not a huge fan of installing test tools in $HOME, especially if the user has no knowledge of that, but we could change the default).

And OpenCoverPath I think is definitely better being explicit. Nobody will ever find OpenCover.xml in their $HOME directory.

SteveL-MSFT · 2018-03-27T23:16:52Z

docs/testing-guidelines/getting-code-coverage.md

+```powershell
+# Collect the coverage data using Get-CodeCoverage from the OpenCover
+# module that was imported above
+PS> $coverageData = Get-CodeCoverage .\coverage.xml


May be worth mentioning that this is an expensive operation so recommendation is to store it in a variable for reuse/re-viewing

markekraus · 2018-03-27T23:58:17Z

docs/testing-guidelines/getting-code-coverage.md

+
+**Note: Code coverage is currently only supported on Windows, since we use OpenCover.**
+
+The PowerShell code base is configured to build with code coverage support using [OpenCover](https://github.com/OpenCover/opencover). You can see the testing coverage of the current [`master`](https://github.com/PowerShell/PowerShell)


Would you be so kind as to switch to semantic linefeeds and use reference links throughout the document?

Example:

The PowerShell code base is configured to build with code coverage support using [OpenCover]. You can see the testing coverage of the current [`master`] branch build at any time at [codecov.io]. [OpenCover]: https://github.com/OpenCover/opencover [`master`]: https://github.com/PowerShell/PowerShell [codecov.io]: https://codecov.io/gh/PowerShell/PowerShell

Becomes:

The PowerShell code base is configured to build with code coverage support using OpenCover. You can see the testing coverage of the current master branch build at any time at codecov.io.

Makes it easier to track changes.

Ah that's a nice feature! Will do.

SteveL-MSFT · 2018-03-28T00:38:23Z

docs/testing-guidelines/getting-code-coverage.md

+
+## Running tests with code coverage analysis
+
+**First**: Open PowerShell in an **elevated** session. OpenCover needs elevated privileges to work.


Second sentence should be on newline. The renderer will put it back.

SteveL-MSFT · 2018-03-28T00:39:02Z

docs/testing-guidelines/getting-code-coverage.md

+PS> Import-Module $PWD\test\tools\OpenCover
+
+# Install OpenCover to a temporary directory
+PS> Install-OpenCover -TargetDirectory $env:TEMP -Force


If the defaults work, I would recommend not adding additional parameters as it makes it more confusing to new users

rjmholt · 2018-03-29T17:53:49Z

I've removed a couple of the parameters given. The rest are the coverage file itself and the OpenCover installation directory. In the case of the code coverage XML, there's no indication of where it is unless the user explicitly states it, so I think better to document it. In the case of the OpenCover installation directory, the default is $HOME, and we don't provide or give directions to clean up afterward. I don't think we should be installing things into $HOME by default and not telling users or cleaning up afterward, so I think for now we should just give them the parameter to set (and recommend the temp directory). I feel like I'm a fairly new user and the most confusing parts to me are where something is not documented or implicit -- if I found OpenCover under ~ a few days later, I would have no idea where it came from or whether I could delete it -- so I'm going to leave those parameters in.

SteveL-MSFT · 2018-04-01T07:47:18Z

@rjmholt separate from this PR, perhaps you can submit a new PR to have OpenCover installed to ~/.opencover which is consistent with where we stick dotnet and rcedit. In general, I would prefer the user not to have to specify parameters where not absolutely necessary as we should have good defaults. If the defaults are not good, let's fix it. If you think there's any confusion to the end user on where things are being put, it would make sense to have a verbose message.

adityapatwardhan · 2018-04-03T17:29:07Z

@rjmholt Please fix spelling errors. See: https://github.com/PowerShell/PowerShell/blob/fc627a1dba69ad839fcb7fe76b9aa4733e2b371f/.github/CONTRIBUTING.md#spellchecking-documentation

adityapatwardhan · 2018-04-03T17:32:24Z

docs/testing-guidelines/getting-code-coverage.md

+
+# Build PowerShell. You may need to add other flags here like
+# -ResGen, -Restore or -PSModuleRestore
+PS> Start-PSBuild -Configuration CodeCoverage


I would add -PSModuleRestore in the example instead of comment, since it will be required for some tests. Maybe even -Clean.

adityapatwardhan · 2018-04-03T17:39:18Z

docs/testing-guidelines/getting-code-coverage.md

+Microsoft.WSMan.Runtime                   80.95  80.33
+Microsoft.PowerShell.Commands.Diagnostics 0      0
+```
+


Compare-CodeCoverage and Compare-FileCoverage might be worth mentioning for comparing two runs.

adityapatwardhan · 2018-04-03T17:40:21Z

docs/testing-guidelines/getting-code-coverage.md

+
+# Finally, open the report in your browser
+Invoke-Item C:\temp\Coverage\index.htm
+```


Also mention Format-FileCoverage for a console visualizer as an alternative.

I tried using Format-FileCoverage, but I'm not sure it does what the help message states. I read through the implementation and it currently only seems to compare two codebases (as in, I couldn't get it to just show me one codebase's test coverage, because the oldBase and newBase parameters not being provided cause the command to fail). I'll do some work on updating that and the other stuff that @SteveL-MSFT mentioned in another PR, and then update the documentation with it.

rjmholt · 2018-04-04T16:23:09Z

@adityapatwardhan Happy to add the extra documentation on the other coverage functions. My documentation is based on this blog post. Is there any documentation I can work off with respect to the other commands you mentioned?

~~Also, I noticed that mdspell is locale-specific - so we should perhaps add the --en-us option to our instructions?~~ Actually, mdspell defaults to en-GB and is planning to change in the next major version. But there is still option support for --en-us.

rjmholt · 2018-04-04T16:45:00Z

mdspell found a few other spelling warnings:

> mdspell.cmd "**/*.md" --ignore-numbers --ignore-acronyms -r
    docs/git/basics.md
       37 | references to our favorites.
       49 | #### Katacoda
       52 | [Git lessons on katacoda](https://www.katacoda.com/cou
       54 | #### Githug
       56 | [Githug](https://github.com/Gazler/gi

    docs/git/README.md
       30 | * Use lowercase-with-dashes for naming.

    docs/learning-powershell/powershell-beginners-guide.md
       44 | interested in the instance of firefox process that are running on y
      134 | ld!" as a file content to the test.txt.
      135 | Because the test.txt file exists already, we use *
      173 | **8. del - Remove-Item**: Deletes the
      175 | ill delete the file /home/jen/test.txt:
      180 | **9. $PSVersionTable**: Displays the version of Po
      182 | Type **$PSVersionTable** in your PowerShell session,
      271 | th PowerShell][remoting] from Channel9
      272 | - [eBooks from PowerShell.org](https://
      272 | - [eBooks from PowerShell.org](https://powershell.org/ebook
      273 | - [eBooks from PowerShell.com][ebooks-p
      273 | - [eBooks from PowerShell.com][ebooks-powershell.com]
      274 | - [eBooks List][ebook-list] by Martin S
      274 | s List][ebook-list] by Martin Schvartzman
      279 | ting in Depth][in-depth] from Channel9
      280 | Management][remote-mgmt] from ITPro

    docs/learning-powershell/README.md
       91 | | pwd                 |pwd, $pwd, G
       91 | | pwd                 |pwd, $pwd, Get-Location      |Sho
       91 | | pwd                 |pwd, $pwd, Get-Location      |Show work
       92 | | clear, Ctrl+L, reset| cls, clear                  |Clea
       93 | | mkdir               |New-Item -Item
       93 | kdir               |New-Item -ItemType Directory |Create a new folde
       94 | | touch test.txt      |New-Item -Path test.txt
       94 | test.txt      |New-Item -Path test.txt      |Create a new empty file
       95 | | cat test1.txt test2.txt         |Get-Conten
       95 | | cat test1.txt test2.txt         |Get-Content test1.tx
       95 | est2.txt         |Get-Content test1.txt, test2.txt       |Display fil
       95 |       |Get-Content test1.txt, test2.txt       |Display files contents
       96 | | cp ./source.txt ./dest/dest.txt |Copy-Item so
       96 | | cp ./source.txt ./dest/dest.txt |Copy-Item source.tx
       96 | | cp ./source.txt ./dest/dest.txt |Copy-Item source.txt dest/de
       96 | xt ./dest/dest.txt |Copy-Item source.txt dest/dest.txt     |Copy a fil
       96 | est.txt |Copy-Item source.txt dest/dest.txt     |Copy a file
       96 | xt |Copy-Item source.txt dest/dest.txt     |Copy a file
       97 | | cp -r ./source ./dest           |Copy-Item ./source
       97 |         |Copy-Item ./source ./dest -Recurse     |Recursively cop
       98 | | mv ./source.txt ./dest/dest.txt
       98 | | mv ./source.txt ./dest/dest.txt |Move-Item ./
       98 | | mv ./source.txt ./dest/dest.txt |Move-Item ./source.
       98 | | mv ./source.txt ./dest/dest.txt |Move-Item ./source.txt ./des
       98 |  ./dest/dest.txt |Move-Item ./source.txt ./dest/dest.txt |Move a file
       98 | txt |Move-Item ./source.txt ./dest/dest.txt |Move a file to othe
       98 | Move-Item ./source.txt ./dest/dest.txt |Move a file to other folder
       99 | | rm test.txt
       99 | | rm test.txt                     |Remove-I
       99 |                  |Remove-Item test.txt                   |Delete a f
      100 | | rm -r &lt;folderName>
      100 | | rm -r &lt;folderName>           |Remove-Item &lt;f
      100 | e>           |Remove-Item &lt;folderName> -Recurse   |Delete a folder
      102 | | grep -Rin "sometext" --include="*.
      102 | | grep -Rin "sometext" --include="*.cs" |
      102 | | grep -Rin "sometext" --include="*.cs" |Get-ChildI
      102 | r> \| Select-String -Pattern "sometext" | Recursively case-insensiti

    docs/learning-powershell/working-with-powershell-objects.md
       64 | To confirm, we can call the GetType() method interactively from t

>> 60 spelling errors found in 68 files

I'll leave those for another PR unless it's preferred I fix them here.

adityapatwardhan · 2018-04-04T17:31:06Z

@rjmholt Just fix the errors introduced in this change.
The for en-GB to en-US should be done in a separate PR.
For more information about other OpenCover cmdlets, Get-Help <cmdletname> -Full

Add code coverage report generation instructions

6b8100f

rjmholt assigned JamesWTruher and SteveL-MSFT Mar 27, 2018

SteveL-MSFT requested a review from JamesWTruher March 27, 2018 22:34

SteveL-MSFT unassigned JamesWTruher and SteveL-MSFT Mar 27, 2018

SteveL-MSFT requested a review from adityapatwardhan March 27, 2018 22:34

SteveL-MSFT assigned adityapatwardhan Mar 27, 2018

SteveL-MSFT requested changes Mar 27, 2018

View reviewed changes

rjmholt added 2 commits March 27, 2018 16:40

Add Publish-PSTestTools in docs

69f82ba

Markdown fixups

c815359

markekraus reviewed Mar 27, 2018

View reviewed changes

rjmholt added 2 commits March 27, 2018 17:10

Use reference links

581cde6

Add semantic line feeds

0f0eebe

SteveL-MSFT requested changes Mar 28, 2018

View reviewed changes

rjmholt added 2 commits March 29, 2018 10:43

Newline for OpenCover elevated privileges

d0119b5

Remove default variables

58b598d

SteveL-MSFT approved these changes Apr 1, 2018

View reviewed changes

adityapatwardhan requested changes Apr 3, 2018

View reviewed changes

rjmholt added 2 commits April 4, 2018 09:34

Update spelling file

17e2521

Fix locale-specific spelling

4246962

Add references to coverage comparison commands

7d51822

adityapatwardhan approved these changes May 4, 2018

View reviewed changes

adityapatwardhan merged commit 5cc27e2 into PowerShell:master May 4, 2018

rjmholt deleted the code-coverage-doc branch June 22, 2018 22:55

		@@ -0,0 +1,127 @@
		Getting Code Coverage Analysis for PowerShell
		===


		Note: Code coverage is currently only supported on Windows, since we use OpenCover

		The PowerShell code base is configured to build with code coverage support using [OpenCover](https://github.com/OpenCover/opencover). You can see the testing coverage of the current [`master`](https://github.com/PowerShell/PowerShell) branch build at any time at [codecov.io](https://codecov.io/gh/PowerShell/PowerShell).


		## Running tests with code coverage analysis

		First: Open PowerShell in an elevated session. OpenCover needs elevated privileges to work.

Conversation

rjmholt commented Mar 27, 2018

PR Summary

PR Checklist

Uh oh!

SteveL-MSFT commented Mar 27, 2018

Uh oh!

rjmholt commented Mar 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markekraus Mar 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjmholt commented Mar 29, 2018

Uh oh!

SteveL-MSFT commented Apr 1, 2018

Uh oh!

adityapatwardhan commented Apr 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjmholt Apr 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjmholt commented Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rjmholt commented Apr 4, 2018

Uh oh!

adityapatwardhan commented Apr 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

markekraus Mar 27, 2018 •

edited

Loading

rjmholt Apr 5, 2018 •

edited

Loading

rjmholt commented Apr 4, 2018 •

edited

Loading