Skip to content

Split IISFunctionals startup/shadow copy tests #38575

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

Merged
merged 9 commits into from
Nov 24, 2021
Merged

Split IISFunctionals startup/shadow copy tests #38575

merged 9 commits into from
Nov 24, 2021

Conversation

HaoK
Copy link
Member

@HaoK HaoK commented Nov 22, 2021

Fixes #37915

Start splitting up tests to try and reduce the long pole of IIS.Functional tests

@ghost ghost added the area-runtime label Nov 22, 2021
@HaoK
Copy link
Member Author

HaoK commented Nov 23, 2021

Splitting the these tests gets IIS.Functionals down to 25 minutes from 33 minutes, I'm not sure its worth moving the HTTP2/3 tests out as it only shaves off about 2 minutes from the running time, but IIS functionals are still the long pole, with the next closest around 18 minutes, so we could shave another 7 minutes from the total time for asnpetcore-ci PR checks by splitting things up more

It gets a bit uglier from here, we'd potentially have to split Startup/Shutdown tests out, which would create 4-5 more test projects since that would require splitting the base tests into 2 groups but would almost certainly get the time down to where we want...

Thoughts @adityamandaleeka @Tratcher @dougbu @sebastienros @wtgodbe

@dougbu
Copy link
Contributor

dougbu commented Nov 23, 2021

@HaoK if I'm reading this correctly, you're currently splitting IIS.FunctionalTests into 3 parts -- tests from Common.FunctionalTests, new IIS.Http.FunctionalTests, and new IIS.ShadowCopy.Tests.

  • I think you're suggesting IIS.Http.FunctionalTests doesn't help much. Is that correct❔
  • I also am pretty sure work items for IIS.FunctionalTests, IIS.Http.FunctionalTests, and IIS.ShadowCopy.Tests don't overlap because the Common.FunctionalTests tests execute only in IIS.FunctionalTests (though the other two reuse the same Infrastructure files). Right❔
  • How long do the work items for each of those take on the supported Helix queues❔
    • Need more than one run to get a solid sense here.
    • The specific Helix queue probably matters. Suggest temporarily updating helix-matrix.yml to limit Helix runs to src/Servers/IIS/IIS/test/**/*.csproj (if that works -- I hope it does) and Helix.Common.props to use only queues to where those tests can run, then doing a few manual aspnetcore-helix-matrix builds.

@HaoK
Copy link
Member Author

HaoK commented Nov 23, 2021

Not quite, I just moved ShadowTests into a separate work item, as well as the handful of Http3/Http2 tests as well. But to get IIS.Functionals all the way down to 17 minutes we will need to split some of the Common.FunctionalTests into a separate, Common.StartupTests or something.

The only queue that matters it he windows 11 queue since that's the only queue these tests run on for the pr checks, there hasn't been basically much variance from what I've seen over 5-6 runs, as they split out wait/delay/queue time from actual machine time, for example

https://helix.dot.net/api/jobs/bf1d596a-36e0-44c0-b19c-19b1bf483897/workitems/IIS.FunctionalTests--net7.0?api-version=2019-06-17

I'm really only asking if we think its worth the additional split up of the shared functional tests into 4 more test projects to cut 7 minutes off our PR runs.

Today we have:
IIS.FunctionalTests
IIS.NewHandler.FunctionalTests
IIS.NewShim.FunctionalTests
IISExpress.FunctionalTests

We'd end up with something like

IIS.FunctionalTests
IIS.NewHandler.FunctionalTests
IIS.NewShim.FunctionalTests
IISExpress.FunctionalTests
IIS.Startup.FunctionalTests
IIS.NewHandler.Startup.FunctionalTests
IIS.NewShim.Startup.FunctionalTests
IISExpress.Startup.FunctionalTests
IIS.Http.FunctionalTests
IIS.ShadowCopy.Tests

@dougbu
Copy link
Contributor

dougbu commented Nov 23, 2021

At the moment, the $(HelixTimeout) for all these projects (from FunctionalTest.props) is 01:15:00. If we're already down below that we're ahead 😺

The only queue that matters it he windows 11 queue since that's the only queue these tests run on for the pr checks

That's not the case if we're considering lowering the above timeout.

@HaoK
Copy link
Member Author

HaoK commented Nov 23, 2021

Last time I looked at the kusto data we didn't have any jobs that took longer than 17 minutes in total other than IIS Functionals, so we can probably start with just 30 minutes timeout everywhere, and just remove the special timeout. Kusto is down right now so I can't check...

Actually it looks like most of the other test projects only include the startup/shutdown tests i was going to split out already, so it looks like we can probably get what we want just by splitting two more test files into IIS.LongTests, I'll give that a shot and see if that gets us below 20 minutes

Comment on lines 14 to 17
<ItemGroup>
<!-- Required for QUIC & HTTP/3 in .NET 6 - https://github.com/dotnet/runtime/pull/55332 -->
<RuntimeHostConfigurationOption Include="System.Net.SocketsHttpHandler.Http3Support" Value="true" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to clean out some unneeded flags and dependencies that don't apply to the tests in these split projects.

@HaoK
Copy link
Member Author

HaoK commented Nov 23, 2021

Okay I think this is the ideal split, since the startup/shutdown tests take 17 minutes, now we have 3 permutations of them which are the long pole, I think can get rid of the Http tests and put those back so we'll end up with:

IIS.FunctionalTests (-ShadowCopy, startup/shutdown), should be < 10 minutes
IIS.ShadowCopy.Tests ~9 min
IIS.NewHandler.FunctionalTests (unchanged, only Startup/Shutdown tests) ~17 min
IIS.NewShim.FunctionalTests (unchanged Startup/Shutdown tests) ~17 min
IIS.LongTests (only startup/shutdown tests) ~17 min

I'll go ahead and change the global helix job timeout to a consistent 30 min as part of this iteration before trying a few runs to see how things look, we should shave around 15 minutes from our average PRs I think

@HaoK HaoK changed the title Split shadow copy tests Split IISFunctionals startup/shadow copy tests Nov 23, 2021
@HaoK HaoK requested review from Tratcher and dougbu November 23, 2021 19:46
@HaoK HaoK marked this pull request as ready for review November 23, 2021 19:46
@HaoK HaoK requested review from BrennanConroy, halter73 and a team as code owners November 23, 2021 19:46
@HaoK
Copy link
Member Author

HaoK commented Nov 23, 2021

Current results:
IIS.Functionals: 6m34s
IIS.LongTests: 17m53s
IIS.NewHandler: 16m
IIS.NewShim: 15m
IIS.ShadowCopy: 9m 44s
IIS.Express: 8m 51s

@Tratcher
Copy link
Member

Nice. What were the times before?

@HaoK
Copy link
Member Author

HaoK commented Nov 23, 2021

Nice. What were the times before?

IIS.Functionals 34m
IIS.NewHandlers, NewShim ~17m (these should be unchanged)

So we should gain around 17 minutes since we queue all the helix workitems at once

@HaoK
Copy link
Member Author

HaoK commented Nov 24, 2021

@HaoK HaoK enabled auto-merge (squash) November 24, 2021 17:15
@HaoK HaoK merged commit e97bc0c into main Nov 24, 2021
@HaoK HaoK deleted the haok/split branch November 24, 2021 19:15
@ghost ghost added this to the 7.0-preview1 milestone Nov 24, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IIS tests are timing out
4 participants