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

Prkrishn/code coverage #2308

Closed
wants to merge 7 commits into from
Closed

Prkrishn/code coverage #2308

wants to merge 7 commits into from

Conversation

pranavkm
Copy link

@pranavkm pranavkm commented Sep 8, 2019

Ignore

@pranavkm pranavkm marked this pull request as ready for review September 8, 2019 06:10
@pranavkm pranavkm requested review from dougbu and a team as code owners September 8, 2019 06:10
@pranavkm
Copy link
Author

pranavkm commented Sep 9, 2019

/azp run all

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@rynowak
Copy link
Member

rynowak commented Sep 10, 2019

Ignore

Don't tell me what to do.

@@ -13,6 +13,7 @@ Param(
[switch] $rebuild,
[switch] $deploy,
[switch][Alias('t')]$test,
[switch] $codeCoverage,
Copy link
Member

Choose a reason for hiding this comment

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

FYI I think you can just pass /p:CodeCoverage=true directly in CIBuild.cmd. This script is auto-updated by arcade and isn't meant to be edited.

Copy link
Author

Choose a reason for hiding this comment

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

I was planning for this to be a target rather than a property. I do have some changes for Arcade, I'll work with them to figure out the right pattern.

@ryanbrandenburg
Copy link

Is this still in active development and/or will it be coming to aspnet/AspNetCore? I think this is super valuable information to have to advise your testing efforts.

@pranavkm
Copy link
Author

pranavkm commented Oct 2, 2019

There's a PR in arcade to make this work - dotnet/arcade#3919 which isn't being actively looked at. I'd be happy if you'd like to drive it.

@dougbu
Copy link
Member

dougbu commented Oct 2, 2019

@pranavkm a few questions

  1. If that PR is ready for review, why is it still a draft?
  2. Are you aware that the PR seems to contain errors sufficient to break the Arcade Helix tests?
  3. Why is it necessary to change the Arcade SDK at all? Seems like your additions should be doable in this repo.

"Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.19458.2",
"Microsoft.DotNet.Helix.Sdk": "2.0.0-beta.19458.2"
"Microsoft.DotNet.Arcade.Sdk": "4.0.0-dev",
"Microsoft.DotNet.Helix.Sdk": "2.0.0-beta.19454.31"
Copy link
Member

Choose a reason for hiding this comment

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

Do not change this portion of the file directly. Need to update the dependencies from Arcade i.e. get darc to change Versions.props and Version.Details.xml as well.

In your branch, execute

darc update-dependencies --source-repo arcade  --channel '.NET Tools - Latest'

or

darc update-dependencies --source-repo arcade  --channel '.NET 3 Tools'

pathtoPublish: artifacts/log/$(_BuildConfig)/codecoverage/
artifactName: $(Agent.Os)_$(Agent.JobName) CodeCoverage
artifactType: Container
parallel: true
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these files included when all of artifacts/log is uploaded?

@@ -108,20 +108,9 @@ stages:
arguments: 'locals all -clear'
- script: eng\common\cibuild.cmd -configuration $(_BuildConfig) -prepareMachine $(_BuildArgs)
displayName: Build and Publish
- script: eng\scripts\ci-flaky-tests.cmd -configuration $(_BuildConfig)
displayName: Run Flaky Tests
continueOnError: true
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right. Why are you removing the flaky test run?

@@ -0,0 +1,18 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

what will reference this new file?

@pranavkm pranavkm closed this Oct 23, 2019
@mmitche mmitche deleted the prkrishn/code-coverage branch January 4, 2021 21:33
@ghost ghost locked as resolved and limited conversation to collaborators May 27, 2023
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.

None yet

5 participants