mysql test timeouts

[danmoseley]

Description

MySql test was timing out here and it seemed almost all the 2 mins was pulling the mysql docker image.

aspire/tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs

Line 156 in 1007a6c

await app.WaitForTextAsync(s_mySqlReadyText).WaitAsync(TimeSpan.FromMinutes(2));

1. Change MySql tests to have a single timeout using a ct, so they're easier to reason about.
2. Change the ct timeout to something similar in terms of a new ExtraLongTimeout, so it can be changed centrally.
3. Make ExtraLongTimeout longer on CI.

It's super hard to reason about whether timeouts are too long or not. They're everywhere in the tests. But it probably makes sense to convert tests to have an overall timeout like this so they're easier to reason about, and to centralize the value to some extent.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • [x ] Yes
  • No
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • Yes
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
      • No
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
    • If yes, have you done a threat model and had a security review?
      • Yes
      • No
  • No
• Does the change require an update in our Aspire docs?
  • Yes
    • Is this introducing a breaking change?
      • Yes
        • Link to aspire-docs issue (please use this breaking-change template):
      • No
        • Link to aspire-docs issue (please use this doc-idea template):
  • No

[Copilot]

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

Files not reviewed (1)

• tests/Aspire.Hosting.MySql.Tests/Aspire.Hosting.MySql.Tests.csproj: Language not supported

Comments suppressed due to low confidence (2)

tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs:69

• [nitpick] Multiplying a TimeSpan directly by an integer is not supported in standard C#. If an operator overload is defined for this, consider making the conversion explicit or introducing a helper method for clarity.

var ct = new CancellationTokenSource(TestConstants.ExtraLongTimeoutTimeSpan * 2).Token;

tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs:69

• [nitpick] Consider defining a named constant (e.g., 'DoubleExtraLongTimeout') instead of multiplying the ExtraLongTimeoutTimeSpan inline. This can improve readability and consistency across tests.

var ct = new CancellationTokenSource(TestConstants.ExtraLongTimeoutTimeSpan * 2).Token;

[danmoseley]

MySql test was timing out here and it seemed almost all the 2 mins was pulling the mysql docker image.

Hmm, except that line is waiting for sql ready before starting the 2 min clock. Hmm, well, still good cleanup.. maybe I misread the logs

[danmoseley]

@radical

[radical]

On CI this would be a 12min timeout, is that intentional?

[danmoseley]

I did this because it was 10 mins already. Now either 6 mins locally or 12 mins in CI. @sebastienros I assume you picked such a large timeout for a reason..?

[sebastienros]

10 minutes? probably to be sure it has enough time to run on the (slow) CI since it starts multiple containers serially. I haven't checked but obviously we can pick whatever the value is from normal runs on the CI + some margin.

[danmoseley]

OK I think this is fine as is -- maybe eventually we have some script that adjusts each test's timeouts to match CI * margin. But right now, most tests don't even have a single timeout.

[danmoseley]

can you sign off if OK?