Skip to content

Conversation

kllysng
Copy link
Contributor

@kllysng kllysng commented Oct 29, 2024

The test is passing locally without issue. Running an ADO PR build to ensure it passes there too.

Update - test passed in ADO build with no issues, running again

Update again - passed again

@kllysng kllysng requested a review from a team as a code owner October 29, 2024 23:13
@msbw2
Copy link
Contributor

msbw2 commented Oct 30, 2024

I think this might be a build step. At least it would make sense to be one to me instead of a unit test.

@keegan-caruso
Copy link
Contributor

I think this might be a build step. At least it would make sense to be one to me instead of a unit test.

Copy link
Contributor

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @kellyyangsong

Do we understand why it was skipped in the first place?

@kllysng
Copy link
Contributor Author

kllysng commented Oct 30, 2024

LGTM Thanks @kellyyangsong

Do we understand why it was skipped in the first place?

@jmprieur yes, thanks for asking, I should have explained in a comment. It was skipped because it would timeout which for this test is taking longer than 3 min in the ADO build. I re-ran this many times and never saw it take longer than 50s to complete.

@kllysng
Copy link
Contributor Author

kllysng commented Oct 30, 2024

I think this might be a build step. At least it would make sense to be one to me instead of a unit test.

I think this might be a build step. At least it would make sense to be one to me instead of a unit test.

We have an analyzer that will cause warnings - but I still think this test is useful until we start treating warnings as errors.

@kllysng kllysng merged commit 594ce69 into dev Oct 30, 2024
6 checks passed
@kllysng kllysng deleted the kellysong/unskip-EnsureAotCompatibility branch October 30, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants