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

Add xunit timeout to hanging CancelledBuild test #5827

Merged
merged 1 commit into from
Oct 26, 2020

Conversation

rainersigwald
Copy link
Member

This was attempted in #5290 but we hit a hang in this test again this week (https://dev.azure.com/dnceng/public/_build/results?buildId=860511).

@rainersigwald rainersigwald added the Area: Our Own Build Problems affecting the build or build infrastructure of the MSBuild repo itself. label Oct 22, 2020
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Looks like we also hung in https://dev.azure.com/dnceng/public/_build/results?buildId=845339&view=results with windows full, but we weren't planning to merge it yet, so we didn't investigate. That means there it can happen on both full and core. I'm wondering if there are multiple places it can hang or if the timeout I added just doesn't work sometimes.

In any case, I'd approve but I think the Timeout parameter for Facts is being deprecated. See here. The most upvoted answer in that thread has an alternate method.

@rainersigwald
Copy link
Member Author

In any case, I'd approve but I think the Timeout parameter for Facts is being deprecated. See here. The most upvoted answer in that thread has an alternate method.

As mentioned in the comments to that answer, this is no longer true. See xunit/xunit@dabc047. Our tests are single-threaded so we can use it.

Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

The later comment didn't sound terribly confident or include a link. It does look like that commit hasn't been reverted, though, so that's good.

Even though we aren't going for parallelizing it, async still would put it on a separate thread, right? Without having looked carefully at how the timeout is implemented, I'm wondering how/whether that works.

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Oct 23, 2020
@rainersigwald
Copy link
Member Author

Branch isn't quite open yet but I need to spin a new build so I'm going to optimistically merge this.

@rainersigwald rainersigwald merged commit fc27b05 into dotnet:master Oct 26, 2020
@rainersigwald rainersigwald deleted the CancelledBuild-timeout branch October 26, 2020 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Our Own Build Problems affecting the build or build infrastructure of the MSBuild repo itself. merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants