-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Net8 tests #7319
Net8 tests #7319
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7319 +/- ##
=========================================
Coverage 68.87% 68.88%
=========================================
Files 1470 1473 +3
Lines 274005 270771 -3234
Branches 28403 27884 -519
=========================================
- Hits 188727 186510 -2217
+ Misses 77959 76987 -972
+ Partials 7319 7274 -45
Flags with carried forward coverage won't be shown. Click here to find out more.
|
https://dev.azure.com/dnceng-public/public/_build/results?buildId=892728&view=results seems yml parsing is not happy. |
- script: ${{ parameters.MsBuildScript}} | ||
$(Build.SourcesDirectory)/eng/helix.proj | ||
/t:Restore | ||
/bl:$(Build.SourcesDirectory)/artifacts/log/${{ parameters.Configuration }}/SendToHelix.binlog |
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.
This uses the same log as below, also most of these parameters are not needed for restore. I imagine this step is just temporary until we refactor in a follow up PR though.
eng/helix.proj
Outdated
@@ -149,16 +161,21 @@ | |||
<Delete Condition="$(HelixTargetQueues.ToLowerInvariant().Contains('ubuntu'))" | |||
Files="@(WindowsFiles);@(OsxFiles)" /> | |||
|
|||
<GenerateFileFromTemplate TemplateFile="$(MSBuildThisFileDirectory)testing\$(RunTestScript)" | |||
OutputPath="$(TargetDir)$(RunTestScript)" |
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.
This TargetDir is not correct. In my PR this worked because I would generate in the context of each test project, so TargetDir would point to the output of the test. Here you are running in the context of helix.proj
so targetdir is the output directory of that one project. I think you need to instead generate into the test's directory, or somehow copy there.
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.
Copilot reviewed 191 out of 206 changed files in this pull request and generated no comments.
Files not reviewed (15)
- build/Codecoverage.proj: Language not supported
- docs/samples/Microsoft.ML.AutoML.Samples/Microsoft.ML.AutoML.Samples.csproj: Language not supported
- docs/samples/Microsoft.ML.Samples.GPU/Microsoft.ML.Samples.GPU.csproj: Language not supported
- docs/samples/Microsoft.ML.Samples.OneDal/Microsoft.ML.Samples.OneDal.csproj: Language not supported
- docs/samples/Microsoft.ML.Samples/Microsoft.ML.Samples.csproj: Language not supported
- eng/Versions.props: Language not supported
- eng/helix.proj: Language not supported
- eng/testing/runTests.cmd: Language not supported
- eng/testing/runTests.sh: Language not supported
- global.json: Language not supported
- src/Microsoft.Data.Analysis.Interactive/Microsoft.Data.Analysis.Interactive.csproj: Language not supported
- src/Microsoft.Data.Analysis/Microsoft.Data.Analysis.csproj: Language not supported
- src/Microsoft.ML.AutoML.Interactive/Microsoft.ML.AutoML.Interactive.csproj: Language not supported
- src/Microsoft.ML.AutoML/Microsoft.ML.AutoML.csproj: Language not supported
- src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt: Language not supported
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, I reviewed projects / targets / source. I trust all the other baseline changes make sense.
Updating tests to work on net8