Skip to content

Set certificate in some Kestrel tests to avoid global machine state #21516

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

Merged
merged 1 commit into from
May 6, 2020

Conversation

BrennanConroy
Copy link
Member

There were some changes to dev certs which is causing hangs in Kestrel tests in #21017. This is because the tests were relying on the dev cert being allowed globally on the test machine, when they should be providing a cert so they don't rely on global state.

Also added a little bit extra test coverage to the existing tests.

@Pilchie
Copy link
Member

Pilchie commented May 5, 2020

👀

@dougbu
Copy link
Contributor

dougbu commented May 5, 2020

👁️ have a question: What's the advantage of an 👀 question over an 👀 reaction to the PR description and clicking the Subscribe button❔ (I understand just clicking Subscribe is invisible and we often want people to know we're looking.)

@BrennanConroy
Copy link
Member Author

Putting 👀 (or any comment) will add you to the mentions in the email metadata, so you can easily filter issues that you're actually interested in. Kevin told me once that he was subscribed to everything and I am too, I know that I have filters based on the email metadata so adding those kinds of comments are super useful for getting updates.

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.

Should have said :shipit:

Unfortunately https://netcorenativeassets.blob.core.windows.net seems to have been flaky during the build. See also #20577 though a retry will be needed here.

@Pilchie
Copy link
Member

Pilchie commented May 5, 2020

Yep, @BrennanConroy nailed why I use 👀. I get email for anything I'm "participating", but see the rest of my notifications in the GitHub notifications UI.

@BrennanConroy BrennanConroy merged commit 88dbfaa into master May 6, 2020
@BrennanConroy BrennanConroy deleted the brecon/https branch May 6, 2020 02:18
@javiercn
Copy link
Member

javiercn commented May 6, 2020

We probably want to port this back to 3.1 too

@dougbu
Copy link
Contributor

dougbu commented May 6, 2020

We probably want to port this back to 3.1 too

No disagreement here. It's a test-only change and can therefore be merged whenever the branch is open. @BrennanConroy

Same thing for d640937 aka #21493 @javiercn

@BrennanConroy
Copy link
Member Author

Sure, I'll make a PR for this one

@javiercn
Copy link
Member

javiercn commented May 6, 2020

Same thing for d640937 aka #21493 @javiercn

I already have a PR out for it #21496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants