Skip to content

Fix unquarantined test #23356

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.

Sign up for GitHub

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

Jump to bottom
Merged
1 commit merged into from
Jun 26, 2020
Merged

Fix unquarantined test #23356

1 commit merged into from
Jun 26, 2020

Conversation

JunTaoLuo
Copy link
Contributor

This will fix the InMemory.FunctionalTests on Win7 and Win 8.1

Fix unquarantined test
fb02760 
This test was unquarantined but needed a fix to pass on Win7
@JunTaoLuo JunTaoLuo requested a review from jkotalik June 25, 2020 18:10
@ghost ghost added the area-servers label Jun 25, 2020
jkotalik
jkotalik approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@jkotalik jkotalik left a comment

Choose a reason for hiding this comment

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

Thanks!

@ghost
Copy link

ghost commented Jun 25, 2020

Hello @JunTaoLuo!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 60 minutes, a condition that will be fulfilled in about 48 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

dougbu
dougbu approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

This is mildly reasonable but the affected test should have been quarantined a while ago. I'm also not positive we should be "fixing" tests without the usual 30d quarantine / release process.

@Tratcher
Copy link
Member

This is mildly reasonable but the affected test should have been quarantined a while ago. I'm also not positive we should be "fixing" tests without the usual 30d quarantine / release process.

Apparently the quarantine doesn't run down-level tests? This was unquarantined because we thought it was passing?

@JunTaoLuo
Copy link
Contributor Author

Correct

@jkotalik jkotalik mentioned this pull request Jun 26, 2020
Merged
@ghost ghost merged commit ef1e47d into master Jun 26, 2020
@ghost ghost deleted the JunTaoLuo-patch-1 branch June 26, 2020 07:18
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@halter73 halter73 halter73 approved these changes

@Tratcher Tratcher Tratcher approved these changes

@dougbu dougbu dougbu approved these changes

@jkotalik jkotalik jkotalik approved these changes

Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JunTaoLuo @Tratcher @halter73 @dougbu @jkotalik @amcasey