Make ConvertTo-Html work on CoreCLR by jeffbi · Pull Request #2241 · PowerShell/PowerShell

jeffbi · 2016-09-12T21:54:39Z

Part of issue #2240

This also includes some Pester test for the cmdlet.

msftclas · 2016-09-12T21:54:43Z

Hi @jeffbi, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

vors · 2016-09-12T22:00:21Z

test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Html.Tests.ps1

@@ -0,0 +1,111 @@
+Describe "ConvertTo-Html DRT Unit Tests" -Tags "CI" {


We are trying to move away from the Windows BVT/DRT terminology.
Please, just use the descriptive names for the Describe. More testing guidelines are here: https://github.com/PowerShell/PowerShell/blob/master/docs/testing-guidelines/WritingPesterTests.md#writing-pester-tests

vors · 2016-09-12T22:46:54Z

Hi @jeffbi ! Thank you for your contribution.

Based on CI logs from appveyor, looks like you hit the problem that was discussed here #2182 (comment)

Multi-line strings are not very reliable for testing, because they depend on OS and the way how test file itself was checkout (for instance on AppVeyor all line ends for git files are \n, not \r\n).

In general, it's better to avoid using multi-line lines in the tests.
This case seems like an exception, where it's reasonable.
You can add code to strip-out line endings, similar to this:

https://github.com/PowerShell/platyPS/blob/32cc4b625348e40ce39dd12da352eabc203720f9/test/Pester/PlatyPs.Tests.ps1#L612

[-] Test ConvertTo-Html with no parameters 100ms
   Expected string length 386 but was 396. Strings differ at index 110.
   Expected: {<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"  "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">\n<html xmlns="http://www.w3.org/1999/xhtml">\n<head>\n<title>HTML TABLE</title>\n</head><body>\n<table>\n<colgroup><col/><col/><col/></colgroup>\n<tr><th>Name</th><th>Age</th><th>Friends</th></tr>\n<tr><td>John Doe</td><td>42</td><td>System.Object[]</td></tr>\n</table>\n</body></html>}
   But was:  {<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"  "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">\r\n<html xmlns="http://www.w3.org/1999/xhtml">\r\n<head>\r\n<title>HTML TABLE</title>\r\n</head><body>\r\n<table>\r\n<colgroup><col/><col/><col/></colgroup>\r\n<tr><th>Name</th><th>Age</th><th>Friends</th></tr>\r\n<tr><td>John Doe</td><td>42</td><td>System.Object[]</td></tr>\r\n</table>\r\n</body></html>}

mirichmo · 2016-09-13T20:50:59Z

test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Html.Tests.ps1

@@ -0,0 +1,111 @@
+Describe "ConvertTo-Html DRT Unit Tests" -Tags "CI" {
+    $customObject = [pscustomobject]@{"Name" = "John Doe"; "Age" = 42; "Friends" = ("Jack", "Jill")}
+    $newLine = [System.Environment]::NewLine


This test code should go within BeforeAll instead of existing within the Describe. See Pester Do's and Don'ts

Done. New commit pushed up.

From: Mike Richmond [mailto:[email protected]]
Sent: Tuesday, September 13, 2016 1:51 PM
To: PowerShell/PowerShell [email protected]
Cc: Jeff Bienstadt [email protected]; Mention [email protected]
Subject: Re: [PowerShell/PowerShell] Make ConvertTo-Html work on CoreCLR (#2241)

In test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Html.Tests.ps1#2241 (comment):

@@ -0,0 +1,111 @@

+Describe "ConvertTo-Html DRT Unit Tests" -Tags "CI" {

$customObject = [pscustomobject]@{"Name" = "John Doe"; "Age" = 42; "Friends" = ("Jack", "Jill")}

$newLine = [System.Environment]::NewLine

This test code should go within BeforeAll instead of existing within the Describe. See Pester Do's and Don'tshttps://github.com/PowerShell/PowerShell/blob/master/docs/testing-guidelines/PesterDoAndDont.md

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/2241/files/e8481e6a53c1d6d2e84c09dc22c2fa94fa8dcf67#r78641559, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGZehET8cnXNuO1cSEqZ2TNnZ-g7Ffa6ks5qpwy7gaJpZM4J7Etq.

msftclas · 2016-09-14T17:33:48Z

@jeffbi, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

mirichmo · 2016-09-14T23:27:57Z

LGTM.

I'll merge it once we get resolution on the CLA issue.

jeffbi · 2016-09-14T23:59:03Z

The CLA has been signed. Is there still an issue with it?

mirichmo

Blocking this PR pending resolution of the CLA issue

msftclas · 2016-09-21T22:50:21Z

Hi @jeffbi, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Make ConvertTo-Html work on CoreCLR

e8481e6

This also includes some Pester test for the cmdlet.

msftclas added the cla-required label Sep 12, 2016

vors reviewed Sep 12, 2016
View reviewed changes

vors self-assigned this Sep 12, 2016

vors added the Review - Waiting on Author label Sep 12, 2016

mirichmo reviewed Sep 13, 2016
View reviewed changes

msftclas added cla-signed and removed cla-required labels Sep 14, 2016

TravisEz13 added Review - Needed and removed Review - Waiting on Author labels Sep 14, 2016

Update test per code review

63bb074

mirichmo assigned mirichmo and unassigned vors Sep 14, 2016

mirichmo suggested changes Sep 15, 2016

View reviewed changes

mirichmo approved these changes Sep 21, 2016

View reviewed changes

mirichmo closed this Sep 21, 2016

mirichmo reopened this Sep 21, 2016

msftclas added the cla-already-signed label Sep 21, 2016

mirichmo merged commit 5c88dbf into PowerShell:master Sep 22, 2016

jeffbi deleted the convertto-html-coreclr branch September 22, 2016 22:53

mirichmo mentioned this pull request Jul 17, 2017

Fix ConvertTo-Html single matched column header bug #4276

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make ConvertTo-Html work on CoreCLR#2241

Make ConvertTo-Html work on CoreCLR#2241
mirichmo merged 2 commits intoPowerShell:masterfrom
jeffbi:convertto-html-coreclr

jeffbi commented Sep 12, 2016

Uh oh!

msftclas commented Sep 12, 2016

Uh oh!

vors Sep 12, 2016

Uh oh!

vors commented Sep 12, 2016

Uh oh!

mirichmo Sep 13, 2016

Uh oh!

jeffbi Sep 14, 2016

Uh oh!

msftclas commented Sep 14, 2016

Uh oh!

mirichmo commented Sep 14, 2016

Uh oh!

jeffbi commented Sep 14, 2016 •

edited

Loading

Uh oh!

mirichmo left a comment

Uh oh!

msftclas commented Sep 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		@@ -0,0 +1,111 @@
		Describe "ConvertTo-Html DRT Unit Tests" -Tags "CI" {

Conversation

jeffbi commented Sep 12, 2016

Uh oh!

msftclas commented Sep 12, 2016

Uh oh!

vors Sep 12, 2016

Choose a reason for hiding this comment

Uh oh!

vors commented Sep 12, 2016

Uh oh!

mirichmo Sep 13, 2016

Choose a reason for hiding this comment

Uh oh!

jeffbi Sep 14, 2016

Choose a reason for hiding this comment

Uh oh!

msftclas commented Sep 14, 2016

Uh oh!

mirichmo commented Sep 14, 2016

Uh oh!

jeffbi commented Sep 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mirichmo left a comment

Choose a reason for hiding this comment

Uh oh!

msftclas commented Sep 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jeffbi commented Sep 14, 2016 •

edited

Loading