Skip to content

Add code coverage build def. #2166

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

Closed
wants to merge 8 commits into from
Closed

Add code coverage build def. #2166

wants to merge 8 commits into from

Conversation

codemzs
Copy link
Member

@codemzs codemzs commented Jan 16, 2019

Adds a new CI leg just for code coverage. The reason for adding a new leg is not let code coverage process affect PR merge.

@codemzs codemzs requested a review from safern January 16, 2019 22:48

- ${{ if eq(variables['codeCoverageBuild'], 'true') }}:
- phase: ${{ parameters.name }}
variables:
Copy link
Member

Choose a reason for hiding this comment

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

I think variables can be common across the two blocks

Copy link
Member

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

We should confirm that the performance overhead of running code coverage alongside one of the current debug builds is substantial before breaking it out into its own leg. Performance testing is currently blocked while we wait for the next release of coverlet.

@@ -6,7 +6,8 @@ parameters:
customMatrixes: ''

phases:
- phase: ${{ parameters.name }}
- ${{ if ne(variables['codeCoverageBuild'], 'true') }}:
- phase: ${{ parameters.name }}
variables:
Copy link
Member

Choose a reason for hiding this comment

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

You would need to indent everything under these if blocks in this file if you go with the current structure of the phase blocks.

That is why the build is failing:

phase-template.yml (Line: 11, Col: 5, Idx: 199) - (Line: 11, Col: 5, Idx: 199): While parsing a block collection, did not find expected '-' indicator.

@@ -8,7 +8,7 @@ resources:
image: microsoft/dotnet-buildtools-prereqs:centos-7-b46d863-20180719033416

phases:
- ${{ if ne(variables['codeCoverageBuild'], 'true') }}:
${{ if ne(variables['codeCoverageBuild'], 'true') }}:
Copy link
Member

Choose a reason for hiding this comment

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

the - are needed at the beginning of this ifs because phases expect objects to be declared. In yaml you specify an object with - the error you're getting I described it in another comment.

@codemzs
Copy link
Member Author

codemzs commented Jan 17, 2019

@sharwell If the new nuget does not affect the performance much then this PR can be closed. The current nuget is very slow and causes build timeouts. We are expecting the new nuget to be available tomorrow.

CC: @shauheen @eerhardt

@safern safern closed this Jan 17, 2019
@safern safern reopened this Jan 17, 2019
@safern safern closed this Jan 17, 2019
@safern safern reopened this Jan 17, 2019
@safern safern closed this Jan 17, 2019
@shauheen shauheen deleted the ccbuilddef branch January 23, 2019 22:48
@ghost ghost locked as resolved and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants