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

Restore EnsureAotCompatibility test #2951

Merged
merged 1 commit into from
Oct 30, 2024

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