Skip to content

Disable some hosting tests failing on CI#4653

Merged
radical merged 2 commits intomicrosoft:mainfrom
radical:hosting-tests-local
Jun 25, 2024
Merged

Disable some hosting tests failing on CI#4653
radical merged 2 commits intomicrosoft:mainfrom
radical:hosting-tests-local

Conversation

@radical
Copy link
Copy Markdown
Member

@radical radical commented Jun 25, 2024

Issues: #4650 and #4651

Microsoft Reviewers: Open in CodeFlow

@radical radical requested review from eerhardt and joperezr June 25, 2024 18:41
@ghost ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Jun 25, 2024
}

[Fact]
[ActiveIssue("https://github.com/dotnet/aspire/issues/4651", typeof(PlatformDetection), nameof(PlatformDetection.IsRunningOnCI))]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why isn't this

Suggested change
[ActiveIssue("https://github.com/dotnet/aspire/issues/4651", typeof(PlatformDetection), nameof(PlatformDetection.IsRunningOnCI))]
[ActiveIssue("https://github.com/dotnet/aspire/issues/4651")]

?

If a test is flaky, I don't want it to fail on my machine locally only to realize it might be broken and not due to my changes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was working fine earlier on local runs, and CI. But started to fail around the time we enabled docker based tests to run on the build machine. But I think this affected by other tests that run before this.

Instead of disabling this one, I will revert all the tests in tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs which had [LocalOnly*] back to being local-only. This is what @mitchdenny suggested in #4142 (comment) too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I retained the attributed as-is on this test because it was also marked LocalOnly* earlier.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@eerhardt I haven't heard from anyone about this test failing locally. But I can disable it for local too, if that's preferable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Merging this to get a green CI. But I can make this change in a follow up PR if you want.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I haven't heard from anyone about this test failing locally.

My guess is that it isn't getting run as often locally as it is in CI.

My thinking is that CI machines are just normal computers. If we have races / flaky failures in our tests on CI, chances are we do locally too.

… run local only

Some tests were enabled to run on CI in
fb97eda but they might be responsible
for some intermittent failures. This commit disables them till the issue
can be fixed.
@radical radical merged commit 2022c81 into microsoft:main Jun 25, 2024
@radical radical deleted the hosting-tests-local branch June 25, 2024 21:01
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-integrations Issues pertaining to Aspire Integrations packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants