GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add File Coverage to coverage data by JamesWTruher · Pull Request #4556 · PowerShell/PowerShell

Conversation

James Truher (JamesWTruher) commented Aug 11, 2017

Copy link

Copy Markdown

Collaborator

It is now possible to see the coverage based on the file and then format
the data so a report generator is not needed. You can map the files from
one location to another, so if you have a copy of the repo, but in a different
location, you can still format the output in a way so it may be studied

Also, help is available for Format-FileCoverage!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) assigned Aditya Patwardhan (adityapatwardhan)

Aug 11, 2017

James Truher (JamesWTruher) requested review from Travis Plunk (TravisEz13) and Aditya Patwardhan (adityapatwardhan)

August 11, 2017 19:58

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label

Aug 11, 2017

James Truher (JamesWTruher) force-pushed the jameswtruher/FileCoverage1 branch from f1f2aa4 to f96d5e4 Compare

August 11, 2017 21:12

James Truher (JamesWTruher) commented Aug 11, 2017

Copy link

Copy Markdown

Collaborator Author

the test failures seem to be flaky tests

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Travis Plunk (TravisEz13) commented Aug 11, 2017

Copy link

Copy Markdown

Member

I've triggered AppVeyor to run again

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) requested changes

Aug 11, 2017

View reviewed changes

Comment thread

test/tools/OpenCover/OpenCover.psm1 Outdated

Aditya Patwardhan (adityapatwardhan) Aug 11, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be have a default value for this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Aug 14, 2017

Copy link

Copy Markdown

Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

under most circumstances (for example, when running on the system where the data was collected), it's not needed at all, so I don't want to do any substitution. By setting it to default of "" and "" (for old and new) no substitution takes place and everything works as expected.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Aug 15, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is precedence here with Invoke-OpenCover, we try to find powershell root directory and build path.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Aug 15, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's the wrong thing to do. This may be used completely outside of a repo unlike invoke-opencover. consider the scenario inspecting coverage on a remote machine where the paths are not the same as what is in the file. so I can say -oldbase f:/github/blah -newwbase \\system\share\blah If you're on the system where you collected the data, you don't to substitute anything, doing a replacement is unneeded. In this scenario (which is quite real, since I work on mac). The path in the log is wrong for me, and my repo may not be the same as where the coverage was gathered. I don't need a lot of complicated, fragile logic when I can just supply it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/tools/OpenCover/OpenCover.psm1 Outdated

Aditya Patwardhan (adityapatwardhan) Aug 11, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use git to find project root for the default value?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Aug 14, 2017

Copy link

Copy Markdown

Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above - generally don't want to change the path, but sometimes I do. setting this to "" makes it work nearly all the time, but when I'm working on my mac which has different path structure, I can still look at the coverage data. In this case I may not be in a repo at all.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Aug 15, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above about precedence in Invoke-OpenCover.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/tools/OpenCover/OpenCover.psm1 Outdated

Aditya Patwardhan (adityapatwardhan) Aug 11, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we always want a verbose message or a Write-Host -Fore Yellow?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Aug 14, 2017

Copy link

Copy Markdown

Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not - removed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) force-pushed the jameswtruher/FileCoverage1 branch from f96d5e4 to 889551a Compare

August 14, 2017 19:59

James Truher (JamesWTruher) commented Aug 14, 2017

Copy link

Copy Markdown

Collaborator Author

Aditya Patwardhan (@adityapatwardhan) updated and pushed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Travis Plunk (TravisEz13) commented Aug 15, 2017

Copy link

Copy Markdown

Member

restarted appveyor due to a myget failure

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) approved these changes

Aug 16, 2017

View reviewed changes

Aditya Patwardhan (adityapatwardhan) left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) commented Aug 16, 2017

Copy link

Copy Markdown

Member

Travis Plunk (@TravisEz13) would you be having a look at this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Travis Plunk (TravisEz13) approved these changes

Aug 17, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) reviewed

Aug 17, 2017

View reviewed changes

Comment thread

test/tools/OpenCover/OpenCover.psm1 Outdated

Dongbo Wang (daxian-dbw) Aug 17, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please fix this indentation?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Aug 17, 2017

Copy link

Copy Markdown

Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

certainly - commit amended and pushed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Aug 17, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dongbo Wang (@daxian-dbw) Can you take a quick look?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) Aug 17, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. #Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Add File Coverage to coverage data

6913de8

It is now possible to see the coverage based on the file and then format
the data so a report generator is not needed. You can map the files from
one location to another, so if you have a copy of the repo, but in a different
location, you can still format the output in a way so it may be studied

James Truher (JamesWTruher) force-pushed the jameswtruher/FileCoverage1 branch from 889551a to 6913de8 Compare

August 17, 2017 19:14

Dongbo Wang (daxian-dbw) approved these changes

Aug 17, 2017

View reviewed changes

Aditya Patwardhan (adityapatwardhan) merged commit f6208f3 into PowerShell:master

Aug 17, 2017

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Add File Coverage to coverage data (PowerShell#4556)

92478ce

It is now possible to see the coverage based on the file and then format
the data so a report generator is not needed. You can map the files from
one location to another, so if you have a copy of the repo, but in a different
location, you can still format the output in a way so it may be studied

Reviewers

Aditya Patwardhan (adityapatwardhan)

adityapatwardhan approved these changes

Dongbo Wang (daxian-dbw)

daxian-dbw approved these changes

Travis Plunk (TravisEz13)

TravisEz13 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Aditya Patwardhan (adityapatwardhan)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

James Truher (JamesWTruher) commented Aug 11, 2017

Uh oh!

James Truher (JamesWTruher) commented Aug 11, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Aug 11, 2017

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!

James Truher (JamesWTruher) Aug 15, 2017 • 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!

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!

James Truher (JamesWTruher) commented Aug 14, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Aug 15, 2017

Uh oh!

Aditya Patwardhan (adityapatwardhan) left a comment

Choose a reason for hiding this comment

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Aug 16, 2017

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!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

James Truher (JamesWTruher) Aug 15, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.