Replace HttpListener Response Tests with WebListener #5540

markekraus · 2017-11-24T21:52:55Z

ref #5464 , #2504, #3267

replaces HttpListener "response" tests with WebListener
Adds /Response/ test to WebListener
removes HtmlWebResponseObject tests as HTML parsing will no longer be done by the web cmdlets Webcmdlets should parse the <html><head><meta charset="foo"> attribute for the correct encoding if not in http header #3267 (comment)
test bodies that used the system new line have been replaced normalized with `n
multiple headers with the same name test enabled for windows (still pending for macOS/Linux)

This should alleviate some of the macOS HttpListener related failures.

markekraus · 2017-11-25T01:35:11Z

AppVeyor CI fail is being addressed by ~~#5542~~ #5554

iSazonov · 2017-11-25T20:11:55Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

Is it ok to use explicit port?

In this case, the port is not used. The header is echoed back like so:

Link: <http://localhost:8080/PowerShell?test=linkheader&maxlinks=5&linknumber=1>; rel="self" Link: <http://localhost:8080/PowerShell?test=linkheader&maxlinks=5&linknumber=2>; rel="next" Link: <http://localhost:8080/PowerShell?test=linkheader&maxlinks=5&linknumber=5>; rel="last"

and then the test is to ensure the RelationLink.Count is 3 and the the exact urls are provided. These URLs could be anything for this test, but since the linkheader API still lives on HttpListener and not WebListener, I kept the URLs as is so they can actually be useful if you manually repro the test.

iSazonov · 2017-11-25T20:13:53Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

Could you please explain why we want to remove these tests?

I did in the PR description.

iSazonov · 2017-11-25T20:18:08Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

We can remove the BeforeAll .

iSazonov · 2017-11-25T20:19:33Z

test/tools/WebListener/Controllers/ResponseController.cs

Please remove the extra line.

iSazonov · 2017-11-25T20:20:47Z

test/tools/WebListener/Controllers/ResponseController.cs

We never use the formatting.

Do i really need to have this all on one line where you have to scroll for an eternity to see it?

I believe we use the pattern:

if (Request.Query.TryGetValue("statuscode", out statusCodes) && Int32.TryParse(statusCodes.FirstOrDefault(), out statusCode)) { ... }

iSazonov · 2017-11-29T05:48:36Z

@markekraus Could you please rebase to pass CI Appveyor?

markekraus · 2017-11-29T10:03:52Z

@iSazonov Rebase wont help this one. It already picked up #5554 in the last empty commit to re-run CI. The problem now is fail in #5567 (which is actually a dup of an older issue where I noted the same behavior randomly happening).

I will rebase anyway in hopes that the WMF links are working now and we don't find some other transient test issue in the feature tests.

daxian-dbw · 2017-11-29T17:58:44Z

The test run got a hard failure:

  WebListener -> /home/travis/build/PowerShell/PowerShell/test/tools/WebListener/bin/Linux/netcoreapp2.0/linux-x64/WebListener.dll
  WebListener -> /home/travis/build/PowerShell/PowerShell/test/tools/WebListener/bin/
Unhandled Exception: System.TypeInitializationException: The type initializer for 'System.Management.Automation.Tracing.PSEtwLog' threw an exception. ---> System.TypeInitializationException: The type initializer for 'System.Management.Automation.Tracing.PSSysLogProvider' threw an exception. ---> System.DllNotFoundException: Unable to load DLL 'psl-native': The specified module or one of its dependencies could not be found.
 (Exception from HRESULT: 0x8007007E)
   at System.Management.Automation.Tracing.NativeMethods.OpenLog(IntPtr ident, SysLogPriority facility)
   at System.Management.Automation.Tracing.SysLogProvider..ctor(String applicationId, PSLevel level, PSKeyword keywords, PSChannel channels)
   at System.Management.Automation.Tracing.PSSysLogProvider..cctor()
   --- End of inner exception stack trace ---
   at System.Management.Automation.Tracing.PSSysLogProvider..ctor()
   at System.Management.Automation.Tracing.PSEtwLog..cctor()
   --- End of inner exception stack trace ---
   at System.Management.Automation.Tracing.PSEtwLog.LogOperationalInformation(PSEventId id, PSOpcode opcode, PSTask task, PSKeyword keyword, Object[] args)
   at Microsoft.PowerShell.UnmanagedPSEntry.Start(String consoleFilePath, String[] args, Int32 argc)
   at Microsoft.PowerShell.ManagedPSEntry.Main(String[] args) in /home/travis/build/PowerShell/PowerShell/src/powershell/Program.cs:line 23

@markekraus I'm not sure how the PSEtwLog came in the play, but it unveils a bug in SysLogProvider.cs, where libpsl-native should be used instead of psl-native. I opened bug #5577 for that.

markekraus · 2017-11-29T18:23:12Z

Is there some reason we don't run the feature tests on every PR? I run into so many CI fails because most of the web cmdlet tests are gated behind the feature tag. Changes made in other PR's that affect the Feature tests are not being properly vetted. In the past even my own PRs have been approved with broken feature tests.

daxian-dbw · 2017-11-29T18:56:49Z

Feature test run takes more time and not every PR needs the full feature test run, so it's not on by default.
It can be turned on with the [Feature] tag, so the maintainer assigned to the PR or any reviewers of the PR can request a feature test run for the PR to be merged.

iSazonov · 2017-11-30T10:28:50Z

I'm not sure how the PSEtwLog came in the play

@dantraMSFT Have you any ideas why the PSEtwLog is activated from WebListener?

markekraus · 2017-11-30T10:38:00Z

PSEtwLog is not being activated from WebListener...
This line

WebListener -> /home/travis/build/PowerShell/PowerShell/test/tools/WebListener/bin/

indicates that the Publish-PSTestTools has completed:

PowerShell/build.psm1

Line 960 in 7dce411

Publish-PSTestTools | ForEach-Object {Write-Host $_}

WebListener is not being run at that point. and the compile appears to be successful

This is failing sometime after Publish-PSTestTools and before the pester tests start. After weblistener is compiled Start-PSPester builds the command line and path then starts a native execution of pwsh. Plenty of places there for PSEtwLog to trigger.

daxian-dbw · 2017-11-30T16:23:33Z

The root cause of the loading failure was that one of our team members uploaded a libpsl-native nuget package that contains a corrupted Linux binary file. The package has been fixed.
As for #5577, .NET Core actually support lack of "lib" prefix in DllImport name (see dotnet/coreclr#1248), but #5580 was merged anyways to make the naming consistent.

markekraus · 2017-11-30T16:29:12Z

thanks. This PR shouldn't have anything outstanding on it.

* Add Response Controller * [Feature] Replace HttpListener Response tests with WebListener

markekraus requested review from TravisEz13, adityapatwardhan and daxian-dbw as code owners November 24, 2017 21:52

markekraus mentioned this pull request Nov 24, 2017

macOS is hanging during webcmdlets tests #5464

Closed

5 tasks

iSazonov reviewed Nov 25, 2017

View reviewed changes

iSazonov approved these changes Nov 27, 2017

View reviewed changes

iSazonov assigned adityapatwardhan Nov 27, 2017

adityapatwardhan approved these changes Nov 27, 2017

View reviewed changes

TravisEz13 approved these changes Nov 28, 2017

View reviewed changes

markekraus added 4 commits November 29, 2017 04:08

Add Response Controller

d33deef

[Feature] Replace HttpListener Response tests with WebListener

97ee405

[Feature] Address PR Feedback

97af3a2

[Feature] Address PR Feedback

26d4ebe

markekraus force-pushed the HttpListenerToWebListener branch from f256997 to 26d4ebe Compare November 29, 2017 10:11

adityapatwardhan merged commit 95a8a64 into PowerShell:master Dec 4, 2017

TravisEz13 added this to the 6.0.0-RC.2 milestone Dec 5, 2017

TravisEz13 pushed a commit to TravisEz13/PowerShell that referenced this pull request Dec 5, 2017

Replace HttpListener Response Tests with WebListener (PowerShell#5540)

0c4f3be

* Add Response Controller * [Feature] Replace HttpListener Response tests with WebListener

TravisEz13 mentioned this pull request Dec 5, 2017

Merge changes from master to 6.0.0-rc.2 #5626

Merged

TravisEz13 pushed a commit that referenced this pull request Dec 5, 2017

Replace HttpListener Response Tests with WebListener (#5540)

7da528c

* Add Response Controller * [Feature] Replace HttpListener Response tests with WebListener

markekraus deleted the HttpListenerToWebListener branch January 19, 2018 18:58

Replace HttpListener Response Tests with WebListener #5540

Replace HttpListener Response Tests with WebListener #5540

Uh oh!

Conversation

markekraus commented Nov 24, 2017

Uh oh!

markekraus commented Nov 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Nov 29, 2017

Uh oh!

markekraus commented Nov 29, 2017

Uh oh!

daxian-dbw commented Nov 29, 2017

Uh oh!

markekraus commented Nov 29, 2017

Uh oh!

daxian-dbw commented Nov 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Nov 30, 2017

Uh oh!

markekraus commented Nov 30, 2017

Uh oh!

daxian-dbw commented Nov 30, 2017

Uh oh!

markekraus commented Nov 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

markekraus commented Nov 25, 2017 •

edited

Loading

daxian-dbw commented Nov 29, 2017 •

edited

Loading