Skip to content

Make CodeCoverage tests work with CodeCov.io and related fixes #4050

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.

Sign up for GitHub

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

Jump to bottom
Merged
TravisEz13 merged 2 commits into from
Jun 22, 2017
Merged

Make CodeCoverage tests work with CodeCov.io and related fixes #4050

TravisEz13 merged 2 commits into from
Jun 22, 2017

Conversation

@adityapatwardhan
Copy link
Member

@adityapatwardhan adityapatwardhan commented Jun 19, 2017

  • Changes to layout of package caused some changes to package path.
  • Added Test modules from tests\tools\modules
  • Fixed Get-ChildItem test
  • Added convertor for converting OpenCover output file to JSON.
  • Updated how the file is uploaded to CodeCov.io

@adityapatwardhan
Fixed OpenCover module and CodeCoverage launcher script
5ebfa56 
* Changes to layout of package caused some changes to package path.
* Added Test modules from tests\tools\modules
* Fixed Get-ChildItem test
* Added convertor for converting OpenCover output file to JSON.
* Updated how the file is uploaded to CodeCov.io
JamesWTruher
JamesWTruher reviewed Jun 19, 2017
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Management/Get-ChildItem.Tests.ps1 Outdated
}
It "Should give .sys file if the fullpath is specified with hidden and force parameter" -Skip:(!$IsWindows){
$file = Get-ChildItem -path "$env:SystemDrive\\pagefile.sys" -Hidden
It "Should find the hidden file if specified with hidden switch" -Skip:(!$IsWindows){
Copy link
Collaborator

@JamesWTruher JamesWTruher Jun 19, 2017

Choose a reason for hiding this comment

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

I'd love to see this written more directly and support on non-Windows. How about this?

    It "Should find the hidden file if specified with hidden switch" {
        $null = new-item $TESTDRIVE/.file.txt # automatically hidden on Linux
        if ( $IsWindows ) {
            attrib +h $TESTDRIVE/.file.txt
        }
        (Get-ChildItem $TESTDRIVE).count | should be 0
        (Get-ChildItem -hidden $TESTDRIVE).Count | should be 1
    }

Copy link
Member Author

@adityapatwardhan adityapatwardhan Jun 19, 2017

Choose a reason for hiding this comment

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

Just removed the Skip, it works on Ubuntu.

JamesWTruher
JamesWTruher reviewed Jun 19, 2017
View reviewed changes
test/tools/CodeCoverageAutomation/Start-CodeCoverageRun.ps1
{
$lineCoverage = @{}
$sequencePoints = $script:covData | Select-Xml ".//SequencePoint[@fileid = `"$fileId`"]"

Copy link
Collaborator

@JamesWTruher JamesWTruher Jun 19, 2017

Choose a reason for hiding this comment

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

does this work with single quotes? If so, then:
".//SequencePoint[@fileid='$fileid']"
should work

Copy link
Member Author

@adityapatwardhan adityapatwardhan Jun 19, 2017

Choose a reason for hiding this comment

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

Fixed.

JamesWTruher
JamesWTruher reviewed Jun 19, 2017
View reviewed changes
test/tools/OpenCover/OpenCover.psm1

$modulePathParts = $env:PSModulePath -split ';'

foreach($part in $modulePathParts)
Copy link
Collaborator

@JamesWTruher JamesWTruher Jun 19, 2017

Choose a reason for hiding this comment

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

foreach [](start = 12, length = 7)

i wonder if this is really a fatal error - if the path doesn't exist should we really fail?

Copy link
Member Author

@adityapatwardhan adityapatwardhan Jun 19, 2017

Choose a reason for hiding this comment

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

I want this to be fatal error so that we do not have unpredictable results due to a missing path.

@JamesWTruher
Copy link
Collaborator

JamesWTruher commented Jun 19, 2017

function Invoke-OpenCover

would it be possible to check for elevated privs before starting the rest? I seem to remember that caught steve off guard

Refers to: test/tools/OpenCover/OpenCover.psm1:421 in 5ebfa56. [](commit_id = 5ebfa56, deletion_comment = False)

JamesWTruher
JamesWTruher approved these changes Jun 20, 2017
View reviewed changes
Copy link
Collaborator

@JamesWTruher JamesWTruher left a comment

Choose a reason for hiding this comment

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

LGTM

@adityapatwardhan
Copy link
Member Author

adityapatwardhan commented Jun 20, 2017

@PowerShell/powershell-maintainers This is ready for merging.

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Jun 20, 2017

@vors can you merge this so we can start getting code coverage data again?

@adityapatwardhan
Copy link
Member Author

adityapatwardhan commented Jun 21, 2017

@TravisEz13 Can you have a look at this and merge?

@TravisEz13
Copy link
Member

TravisEz13 commented Jun 22, 2017

Please make sure that you add a meaningful title per the contribution guide.

@TravisEz13 TravisEz13 changed the title Fixed OpenCover module and CodeCoverage launcher script Make CodeCoverage tests work with CodeCov.io and related fixes Jun 22, 2017
@TravisEz13
Copy link
Member

TravisEz13 commented Jun 22, 2017

I changed the title for you... Hope it's close enough.

@TravisEz13 TravisEz13 merged commit c7b959b into PowerShell:master Jun 22, 2017
@adityapatwardhan adityapatwardhan deleted the CCFixes branch June 22, 2017 20:44
@iSazonov iSazonov mentioned this pull request May 21, 2018
Merged
10 tasks
iSazonov added a commit that referenced this pull request Jun 4, 2018
@iSazonov
Cleanup FileSystem provider (#6909)
57b3d33 
The cleanup is coming from a code review to cleanup psl. Here we clean up the side branch of the code that will allow later to clean up a branch which uses psl.
- IO.FileInfo does not make system calls in constructor. So we can create the object and then use the required attributes without direct call IO.FileInfo.GetAttributes() ( SafeGetFileAttributes() ). This allow us to exclude some p/invoke calls in our code in later cleanups. Also we get unified code for both Windows and Unix.
- Remove SafeGetFileAttributes() and WinSafeGetFileAttributes(). Currently .Net Core support file attributes on all platforms in fastest way and we can remove our workaround. We get a regression in rare case (for files like pagefile.sys). Fix is ready in CoreFX, we get it in 2.1.1. I suggest ignore the regression because this is a very-very rare situation (Get-ChildItem c:\pagefile.sys -Hidden). The .Net Core team was not even able to create an artificial test for such files and uses a real pagefile.sys file for the test. Also the enumeration is still working (dir c:\ -hidden).
- Re-add test which we lost in #4050. The test is pending because of the regression.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@JamesWTruher JamesWTruher JamesWTruher approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vors vors

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@adityapatwardhan @JamesWTruher @SteveL-MSFT @TravisEz13 @vors @msftclas