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

Run libraries package testing on build agent #53905

Merged
merged 8 commits into from
Jun 9, 2021

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Jun 8, 2021

instead of on Helix as with recent changes the entire package testing
doesn't take more than 2 minutes. Helix created a work item per package
test even though it only took seconds and the average wait time for
a client was 10-15min. This should save some machine cycles as we don't
depend on Helix clients anymore.

Also cleaning up how the generated package test projects are restored
and incorporating a fix from Eric St John to not hard code the package
feeds.

Superseding #53837

instead of on Helix as with recent changes the entire package testing
doesn't take more than 2 minutes. Helix created a work item per package
test even though it only took seconds and the average wait time for
a client was 10-15min.

Also cleaning up how the generated package test projects are restored
and incorporating a fix from Eric St John to not hard code the package
feeds.
@ghost
Copy link

ghost commented Jun 8, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

instead of on Helix as with recent changes the entire package testing
doesn't take more than 2 minutes. Helix created a work item per package
test even though it only took seconds and the average wait time for
a client was 10-15min.

Also cleaning up how the generated package test projects are restored
and incorporating a fix from Eric St John to not hard code the package
feeds.

Superseding #53837

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure-libraries

Milestone: -

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for working on this, with the current improvements I agree that it no longer makes sense to use helix for this.

Let's cleanup helix-queues-setup as well and remove instances of allConfigurations there:

# AllConfigurations
- ${{ if eq(parameters.jobParameters.framework, 'allConfigurations') }}:
- Windows.10.Amd64.Server19H1.Open

- ${{ if notIn(parameters.jobParameters.framework, 'allConfigurations', 'net48') }}:

- ${{ if notIn(parameters.jobParameters.framework, 'allConfigurations', 'net48') }}:

@safern
Copy link
Member

safern commented Jun 9, 2021

It seems like you need to adjust the build command for the all configurations leg? I don't see the tests running on the build log.

@ViktorHofer
Copy link
Member Author

Wooosh, finally it's working but I still need an approval :)

  Testing closure for System.Windows.Extensions TFM=net6.0 RID=none
  Testing closure for System.Threading.Tasks.Dataflow TFM=netcoreapp2.0 RID=none
  Testing for duplicate types for System.Threading.Tasks.Dataflow TFM=netcoreapp2.0 RID=none
  Testing closure for System.Windows.Extensions TFM=net6.0 RID=win
  Testing for duplicate types for System.Windows.Extensions TFM=net6.0 RID=none
  Testing for duplicate types for System.Windows.Extensions TFM=net6.0 RID=win
  Testing System.Threading.Tasks.Dataflow runtime on TFM=net462 RIDs=none
  Testing for duplicate types for System.Threading.Tasks.Dataflow TFM=net462 RID=none
  Testing System.Threading.Tasks.Dataflow TFM=netcoreapp3.0
  Testing System.Threading.Tasks.Dataflow runtime on TFM=net461 RIDs=none
  Testing for duplicate types for System.Threading.Tasks.Dataflow TFM=net461 RID=none
  Testing System.Threading.Tasks.Dataflow TFM=netstandard2.0
  Testing System.Threading.Tasks.Dataflow TFM=net6.0
  Testing System.Threading.Tasks.Dataflow runtime on TFM=netstandard2.0 RIDs=none
  Testing closure for System.Threading.Tasks.Dataflow TFM=netstandard2.0 RID=none
  Testing System.Threading.Tasks.Dataflow TFM=netstandard2.1
  Testing for duplicate types for System.Threading.Tasks.Dataflow TFM=netstandard2.0 RID=none
  Testing System.Threading.Tasks.Dataflow runtime on TFM=netcoreapp3.0 RIDs=none
  Testing closure for System.Threading.Tasks.Dataflow TFM=netcoreapp3.0 RID=none
  Testing for duplicate types for System.Threading.Tasks.Dataflow TFM=netcoreapp3.0 RID=none
  Testing System.Threading.Tasks.Dataflow runtime on TFM=netstandard2.1 RIDs=none
  Testing closure for System.Threading.Tasks.Dataflow TFM=netstandard2.1 RID=none
  Testing for duplicate types for System.Threading.Tasks.Dataflow TFM=netstandard2.1 RID=none

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:20:12.37

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for improving this! Love to see a 20 min all configurations build.

@ViktorHofer
Copy link
Member Author

Love to see a 20 min all configurations build.

Kudos to @ericstj who contributed a great deal to achieving that goal.

@ViktorHofer
Copy link
Member Author

Started 2h 16m 23s ago

I feel like our CI build times are out of control. Waiting more than two hours for libraries tests (normal, non-mobile) to complete isn't great...

@ViktorHofer
Copy link
Member Author

Failures are:

@ViktorHofer ViktorHofer merged commit eb9a5ab into dotnet:main Jun 9, 2021
@ViktorHofer ViktorHofer deleted the PackageTestingLocal branch June 9, 2021 20:55
@ericstj
Copy link
Member

ericstj commented Jun 9, 2021

Very cool reduction in total time for this leg (and resource usage):

Before:
image

After:
image

@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants