GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add test WebListener module and tests for Web Cmdlet Certificate Authentication by markekraus · Pull Request #4622 · PowerShell/PowerShell

Conversation

@markekraus

@markekraus Mark Kraus (markekraus) commented Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Closes #4609 and replaces tests for #4546.

This test uses an ASP.NET Core 2.0 app to run a simple service to echo back the client certificate details if one was provided. If a client cert is not provided, the details are empty and the status reports FAILED. The app runs in a Job.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markekraus
Mark Kraus (markekraus) requested review from Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) as code owners August 20, 2017 10:45
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Aug 20, 2017

Copy link
Copy Markdown

Mark Kraus (@markekraus),
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Aug 20, 2017
@iSazonov

Ilya (iSazonov) commented Aug 20, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) See as we prepare TestExe.csproj

PowerShell/build.psm1

Line 780 in b3e7fd3

@{Path="${PSScriptRoot}/test/tools/TestExe";Output="testexe"}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus

Mark Kraus (markekraus) commented Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Thanks, I'll see how I can fit this one in into Publish-PSTestTools

on another note, the mdspell is falsely calling out ASP.NET in my README.md. Is there place to add that to be ignored? Or is there another way to handle it?

edit: I assume i can modify .spelling to address that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 20, 2017

Copy link
Copy Markdown
Collaborator

Yes, it is .spelling - add Asp.Net (case-sensitive).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 20, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) Can we replace our ListenerHttp script with the application (not in the PR)?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus

Mark Kraus (markekraus) commented Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) In theory we could move all of the web tests to this (with some slight modifications, such as adding a Non-HTTPS listener to it), but, it would mean rewriting the existing features of HttpListener into ASP.NET. Or doing something that allowed PowerShell to manage the business logic.

I was already considering switching the Validate Invoke-WebRequest -SkipCertificateCheck tests over to this new listener since the ServerCert fails as a self-signed cert.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov Ilya (iSazonov) assigned Aditya Patwardhan (adityapatwardhan) Aug 20, 2017
@iSazonov

Ilya (iSazonov) commented Aug 20, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) Please keep the PR as small as possibly - it is better to make cleanups later.

If we can move to the new application from HttpListener we should ask the test area experts to this /cc Aditya Patwardhan (@adityapatwardhan) - if it will be approved we'll review the code with the point in mind.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus

Mark Kraus (markekraus) commented Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Should I roll back the baddssl changes? Aside from those, the rest of the changes on this PR are interdependent.

Can you restart the failed Travis CI test? it failed an unrelated test due to httpbin not being unavailable.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) Main rule is to keep PR as small as possible and don't add unneeded changes.
So I believe, yes, we should rollback - only keep the commit for follow PR. Also you can wait conclusion from Aditya Patwardhan (@adityapatwardhan) I asked to lead this change as the test area expert.
Main question now to continue the review is - do we want move our web tests from HttpListener in future or not.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from 39d21a4 to aea9177 Compare August 20, 2017 16:57
iSazonov
Ilya (iSazonov) reviewed Aug 20, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

function Publish-PSTestTools already added the path into PATH environment variable - so we don't need build the full path.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) That is used to pass to the Job scope where the path changes made by the Publish-PSTestTools are not available.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

not available? I wonder. Do you check? Usually env variables is inherited in sub process.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) it turns out you were right. I was able to adjust this and pull the parent path from the command.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe Start-ClientCertificateCheck

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) I tried to match the other private functions in the test already. I originally wanted to use Start-ClientCertificateCheck, but thought I would get a note for not matching the others. I can't win :D

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

My thoughts was about HttpListener style. Maybe somebody else comment this too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If we move the function in HelpersCommon.psm1 it is ok rename it to Start-ClientCertificateCheck.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If we'll use the application in multiple test files we should save time - run it once and check if it already is running.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) I'm open to suggestions on how to implement that. As it stands now, it starts in less than a second. I was just following the current paradigm for how Start-HttpListener is used in the tests. Currently, StartClientCertificateCheck is called once at the beginning of the Invoke-WebReqeust and once at the begining of the Invoke-RestMethod tests. it is stopped at the end of the tests for those describe blocks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

CI VMs is very-very slow. If locally you see no start delay then on CI it is possible the delay will be large. That's why I think it's best to use one tool (remove HttpListener) and run it only once.

how to implement that

Call Invoke-WebRequest and analyze the response.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) As for the less than 1 second start time, I was referring to the start time within the CI VMs. The majority of the overhead is in the build stage, which has now been offloaded to Publih-PSTestTools. I just have the wait in there to prevent a race condition between the job delay and the first test that requires the resource.

I think the current method for ensuring the process is running is fine. It makes sense to use Invoke-WebRequest for Start-HttpListener as there is no other means for ensuring the server is live. But with this I have access to the console output from the server itself saying it is live.

What I was asking about was how to implement it to be running for multiple tests. What mechanism would start it? Start-PSPester?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We want run tests without Start-PSPester as .\file.test.ps1.
I think we should move the code (1-function to start the app and 2-function check that it is running) in HelpersCommon - in first file we detect that the app is not running and start it, in second file we detect that the app is already running and use it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) Ah. I see. Since it may be decided to move all tests to this, would it make sense to create a module of its own, similar to HttpLitsener? If we want to have logic detecting if it is running or not and with possibility of running multiple of these on different ports, it might warrant a class definition to contain the port and job for a specific listener and a module variable to track started listeners. It might also warrant a rename of the app to something more generic than ClientCertificateCheck

For this PR, to keep it small, Moving the start and stop functions to HelpersCommon and renaming them proper Verb-Noun would be a start.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
TravisEz13 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We could convert the app to binary module (and rename it) and get benefits from autoloading. It may require more effort, but it seems worth.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not sure how that would work. This is an executable binary. I've never seen that kind of thing included in a binary module. I'm not saying it's impossible, just that it is beyond me how to do that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@TravisEz13 Travis Plunk (TravisEz13) Aug 21, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Scripts can still autoload. I think the optimization of converting it to an assembly module can be filed as an issue and let Mark Kraus (@markekraus) prove the concept. Unless someone has time to work with him now on writing an assembly based module.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Travis Plunk (@TravisEz13) and Ilya (@iSazonov) Ok, since it appears we may head in the direction of using this for more tests than just the certificate authentication, I have renamed the app HttpsListener and added an HttpsListener module to house the Start-, Stop, and Get- commands. There is a module script variable to track the running listeners so if it is already listening on the requested port the running listener is returned. To keep it always running, I have removed the stop commands form the AfterAll blocks. I think that incorporates everything discussed here except making it a binary module.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe Stop-ClientCertificateCheck.
If we'll use the application in multiple test files we should it once and shouldn't stop it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) same as with Start-ClientCertificateCheck

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It is better use Context {}. I suggest don't mix HTTP and HTTPS tests and put HTTPS test in separate Context {} while we use different tools for them.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) I think we should maybe do that as a separate PR to clean up all of the current HTTPS tests too. Or, I could build the contexts now and move the other HTTPS into them as we update the tests (i.e. tests referencing badssl.com).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You have to edit all HTTPS tests so I believe it is ok to add Context in the PR.
But if you want I agree to follow the file's pattern now and make cleanups later.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'll do a separate followup PR to this one under the guise of HTTPS test adjustments. moving all the tests (including those not touched in this PR) that depend on HTTPS to a single context in each cmdlet section and switching the URL to the new app... assuming this PR get's approved and that is the way we want to go with this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov and TravisEz13 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
Comment thread test/tools/ClientCertificateCheck/Pages/Index.cshtml Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

GitHub mark absent new line at EOF as error and suggest GitHub community to fix it. We already fixed most of our files in the repo. So please add new line at EOF in all files you change until it is really need empty file.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) will fix

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
TravisEz13 reacted with hooray emoji
All reactions
  • 🎉 1 reaction
Comment thread test/tools/ClientCertificateCheck/Pages/_Layout.cshtml Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

New line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) will fix

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/ClientCertificateCheck/Pages/_ValidationScriptsPartial.cshtml Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

New line?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) it lookslike I don't need this file. i will remove it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/ClientCertificateCheck/README.md Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is it actual?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not sure what you asking here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry. If it is not supported on Mac it make sense to remove.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) removed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/ClientCertificateCheck/dockerfile Outdated

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Do we still need the file?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) If we don't want to try this as a docker container, then no. I can remove the docker parts.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 20, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If it is not supported on Mac it make sense to remove.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) I will remove it and .dockerignore as well as the docker info in the readme.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from ce4296a to 9796ce2 Compare August 20, 2017 19:32
@markekraus

Mark Kraus (markekraus) commented Aug 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Hmm... The macOS Build is "passing" but in actuality it is hitting an unhandled exception and exiting without property saying it has failed.

https://travis-ci.org/PowerShell/PowerShell/jobs/266586350#L8280

Edit:

It looks like macOS has a service that binds to port 8443. I will try switching to port 8083.

The bigger mystery to me is how this managed to terminate all tests without pester complaining about it and how this managed to bubble up from a Job process into the parent process and then up to pester.

Edit 2: switching the port to 8083 fixed the issue in the macOS VM.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov
Ilya (iSazonov) reviewed Aug 21, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

  1. We should make the message more clear - maybe "ClientCertificateCheck is not started."
  2. Since we're still throw maybe move the throw into do-while and make the cycle unlimited:
do
    Start-Sleep -Milliseconds 100
    $containerStatus = $Job.ChildJobs[0].Output | Out-String

    if ($containerStatus -match $initCompleteMessage)
    {
        # Application started.
        break;
    }

    if (get-date) -gt $timeOut) 
    {
         throw "ClientCertificateCheck is not started."
    }
while ($true)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) I will update the throw message.

On the other suggestion, I'm not sure it makes sense to introduce flow logic inside a do block when the while is intended to manage the do's exit. I have never been a fan of while ($true) Is there something wrong with the current logic?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No, only my sample looks more clear for me 😄 because we replace two code blocks with single logical block.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

😆 Ok so it's just a difference of style. I think of it as 2 separate actions, 1) waiting for initialization with timeout and 2) cleanup and error reporting.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 24, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

resolved

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We don't use container - please rename the variable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) I will change it to $initStatus

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 24, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

resolved

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 21, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If we move the function in HelpersCommon.psm1 it is ok rename it to Start-ClientCertificateCheck.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
GavinEke
Gavin Eke (GavinEke) reviewed Aug 21, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated

@GavinEke Gavin Eke (GavinEke) Aug 21, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Mark Kraus (@markekraus) slight typo on endregion

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Gavin Eke (@GavinEke) Thanks. I will correct that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 24, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

resolved

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markekraus Mark Kraus (markekraus) mentioned this pull request Aug 21, 2017
Create Native Test Solution for Certificate Authentication #4609
Closed
@markekraus

Mark Kraus (markekraus) commented Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

It looks like the AppVeyor build beta.5-4786 failed due to temporary issue with powershell.myget.org. if someone could restart the build, that would be helpful.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 21, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) It seems I haven't permissions. So you can "rename" last commit and push - it will be restart CI jobs.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from 1d09ecc to 1cbab33 Compare August 21, 2017 11:13
@markekraus

Mark Kraus (markekraus) commented Aug 21, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Now It looks like https://dotnetcli.azureedge.net/dotnet/Sdk/2.0.0/dotnet-dev-osx-x64.2.0.0.tar.gz has disappeared.

edit: rerunning the CI was successful, but Travis CI has not updated the status in the PR due to some issues.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from 1cbab33 to 1dc95f3 Compare August 21, 2017 12:34
@SteveL-MSFT Steve Lee (SteveL-MSFT) assigned Ilya (iSazonov) and unassigned Aditya Patwardhan (adityapatwardhan) Aug 21, 2017
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 21, 2017

Copy link
Copy Markdown
Member

Ilya (@iSazonov) I'm going to assign this to you as you've been involved while Aditya Patwardhan (@adityapatwardhan) is taking time off
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 21, 2017

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT) We need get approve from test area experts because of significant changes in the PR - please ping anybody if Aditya Patwardhan (@adityapatwardhan) is busy.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus
[feature] Address PR Feedback
1a2fd9b
iSazonov
Ilya (iSazonov) reviewed Aug 24, 2017
View reviewed changes
Comment thread test/tools/Modules/WebListener/WebListener.psm1 Outdated
[OutputType([System.Security.Cryptography.X509Certificates.X509Certificate2])]
param ()
process {
$parentPath = Split-Path -parent (get-command WebListener).Path

@iSazonov Ilya (iSazonov) Aug 24, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I meant we can $MyInvocation.MyCommand.Module.ModuleBase

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 24, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ClientCert.pfx is in the WebListener published directory. If I switch to $MyInvocation.MyCommand.Module.ModuleBase I would need to move it to the WebListener module folder. Is that what you would suggest?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@iSazonov) for clarity, are you asking me to move it and update the code or leave it as is?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I agree to move ClientCert.pfx to module folder.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

resolved

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markekraus
[feature] Cleanup and minor fix
d9770ba 
* Enum was not used
* GetStatus() was not accessing the correct property chain
* Added -Test param to make URL generation smoother in test code and to fix double / issues
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch 2 times, most recently from e807b5b to 89cabf5 Compare August 24, 2017 19:35
@markekraus
[feature] More minor fixes
2c3cbd3 
* Https when it matters.
* Expand property... not exclude..
* Remove superfluous and outdated ToString() override
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from 89cabf5 to 2c3cbd3 Compare August 24, 2017 20:41
@markekraus
[Feature] Move ClientCeret.pfx to WebListener Module
ced727c 
* Move the cert
* Adjust Get-WebListenerClientCertificate 
* Remove cert from csproj
* ActionResult -> JsonResult (was mistakenly left as ActionResult during testing)..
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from 5a0ff37 to ced727c Compare August 25, 2017 11:59
iSazonov
Ilya (iSazonov) reviewed Aug 25, 2017
View reviewed changes
Comment thread test/tools/Modules/WebListener/WebListener.psm1 Outdated
{
return $This.Job.JobStateInfo.State
}

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Extra line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

resolved

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/WebListener/Controllers/CertController.cs
Hashtable output = new Hashtable
{
{"Status", "FAILED"}
};

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I still see ';'.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/WebListener/Controllers/CertController.cs
{"IssuerName" , HttpContext.Connection.ClientCertificate.IssuerName.Name},
{"NotAfter" , HttpContext.Connection.ClientCertificate.NotAfter},
{"NotBefore" , HttpContext.Connection.ClientCertificate.NotBefore}
};

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I still see ';'.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The Semicolon has to be there. When I tried to remove it I get compile errors saying it is missing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry. Closed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/WebListener/WebListener.csproj Outdated
</ItemGroup>

<ItemGroup>
<None Include="ServerCert.pfx" CopyToPublishDirectory="PreserveNewest" />

@iSazonov Ilya (iSazonov) Aug 25, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe put it near client certificate and remove the group?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

resolved

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iSazonov

Ilya (iSazonov) commented Aug 25, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) Thank you for your patience and your speedy work.

LGTM.
markekraus and SteveL-MSFT reacted with hooray emoji
All reactions
  • 🎉 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus

Mark Kraus (markekraus) commented Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Thanks for being patient with me too and for all your suggestions!
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus
[Feature] Move ServerCert.pfx to Module
b93674e 
* Move cert
* Upate csproj
* Update module
* Add/Update README.md's

CI Retest.
@markekraus
Mark Kraus (markekraus) force-pushed the ClientCertAuthTest branch from 4664b1c to b93674e Compare August 25, 2017 15:06
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 25, 2017

Copy link
Copy Markdown
Member

Ilya (@iSazonov) can you formally approve?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov
Ilya (iSazonov) approved these changes Aug 25, 2017
View reviewed changes
@iSazonov

Ilya (iSazonov) commented Aug 25, 2017

Copy link
Copy Markdown
Collaborator

It will be good to get a review from test area experts.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
TravisEz13
Travis Plunk (TravisEz13) approved these changes Aug 25, 2017
View reviewed changes
@iSazonov

Ilya (iSazonov) commented Aug 25, 2017

Copy link
Copy Markdown
Collaborator

Thanks! I'll merge on next week.
markekraus reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov Ilya (iSazonov) changed the title [Feature] Add Tests for Web Cmdlet Certificate Authentication Add test WebListener module and tests for Web Cmdlet Certificate Authentication Aug 31, 2017
@iSazonov
Ilya (iSazonov) merged commit 3b2d169 into PowerShell:master Aug 31, 2017
@iSazonov

Ilya (iSazonov) commented Aug 31, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) Many thanks for great work!

The PR was merged and way is opened for follow work:

  • - Cleanup HTTPS tests, replace #region with "Context".
  • - Cleanup HTTP test to use WebListener.

Please move the points to a tracking Issue.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus Mark Kraus (markekraus) mentioned this pull request Aug 31, 2017
Move WebCmdlet Tests relying on HTTPS to WebListener Module #4719
Closed
@markekraus

Mark Kraus (markekraus) commented Aug 31, 2017

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) I have opened #4179 to track the HTTPS tests.

Should I continue the work for the second item under the already open #2504 or should I create a new issue?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 31, 2017

Copy link
Copy Markdown
Collaborator

Mark Kraus (@markekraus) #2504 is good. Please leave a message about WebListener there for cross reference. And if you want work on this you should ask Travis there.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw Dongbo Wang (daxian-dbw) mentioned this pull request Aug 31, 2017
Add explicit ContentType detection to Invoke-RestMethod #4692
Merged
@markekraus
Mark Kraus (markekraus) deleted the ClientCertAuthTest branch September 22, 2017 20:36
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@markekraus @iSazonov
Add test WebListener module and tests for Web Cmdlet Certificate Auth…
b8b49e4 
…entication (PowerShell#4622)

Introduce new test module 'WebListener.psm1'.
Now web HTTPS tests can use it to exclude using external sites.

PowerShell#4609

* [Feature] Add Tests for Web Cmdlet Certificate Authentication

PowerShell#4609

* [feature] Add new app to Publish-PSTestTools refactor tests

also add ASP.NET to .spelling

* [feature] spelling fix

* [feature] revert badssl changes

* [feature] Impliment suggestions

* [feature] Spelling, var rename, port 8443 to 8083

rebase fix conflict

* [feature] Rename to HttpsListener and Module-ize

.

* [feature] password protect ClientCert to fix macOS import issue

* [feature] Rename to WebListener 

* Rename HttpsListener to WebListener
* Switch Listener from Razor pages to MVC
* Address PR feedback
* Adjust tests

* [feature] Address PR feedback

* [feature] Replace missing smeicolons

* [feature] Address PR Feedback

* [feature] Cleanup and minor fix

* Enum was not used
* GetStatus() was not accessing the correct property chain
* Added -Test param to make URL generation smoother in test code and to fix double / issues

* [feature] More minor fixes

* Https when it matters.
* Expand property... not exclude..
* Remove superfluous and outdated ToString() override

* [Feature] Move ClientCeret.pfx to WebListener Module

* Move the cert
* Adjust Get-WebListenerClientCertificate 
* Remove cert from csproj
* ActionResult -> JsonResult (was mistakenly left as ActionResult during testing)..

* [Feature] Move ServerCert.pfx to Module

* Move cert
* Upate csproj
* Update module
* Add/Update README.md's

CI Retest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 approved these changes
@iSazonov Ilya (iSazonov) iSazonov approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
+1 more reviewer
@GavinEke Gavin Eke (GavinEke) GavinEke left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@iSazonov Ilya (iSazonov)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@markekraus @msftclas @iSazonov @SteveL-MSFT @TravisEz13 @GavinEke @adityapatwardhan
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.