Skip to content

Make ConvertTo-Html work on CoreCLR - #2241

Merged
Mike Richmond (mirichmo) merged 2 commits into
PowerShell:masterfrom
jeffbi:convertto-html-coreclr
Sep 22, 2016
Merged

Make ConvertTo-Html work on CoreCLR#2241
Mike Richmond (mirichmo) merged 2 commits into
PowerShell:masterfrom
jeffbi:convertto-html-coreclr

Conversation

@jeffbi

@jeffbi jeffbi commented Sep 12, 2016

Copy link
Copy Markdown

Part of issue #2240

This also includes some Pester test for the cmdlet.

Jeff Bienstadt
Make ConvertTo-Html work on CoreCLR
e8481e6 
This also includes some Pester test for the cmdlet.
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Sep 12, 2016

Copy link
Copy Markdown

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
vors reviewed Sep 12, 2016
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Html.Tests.ps1 Outdated
@@ -0,0 +1,111 @@
Describe "ConvertTo-Html DRT Unit Tests" -Tags "CI" {

@vors sergei (vors) Sep 12, 2016

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 sergei (vors) self-assigned this Sep 12, 2016
@vors

sergei (vors) commented Sep 12, 2016

Copy link
Copy Markdown
Collaborator

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
mirichmo reviewed Sep 13, 2016
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Html.Tests.ps1 Outdated
@@ -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

@mirichmo Mike Richmond (mirichmo) Sep 13, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@jeffbi jeffbi Sep 14, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. New commit pushed up.

From: Mike Richmond [mailto:notifications@github.com]
Sent: Tuesday, September 13, 2016 1:51 PM
To: PowerShell/PowerShell PowerShell@noreply.github.com
Cc: Jeff Bienstadt JeffBi@jetstreamsoftware.com; Mention mention@noreply.github.com
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

Microsoft Contribution License Agreements (msftclas) commented Sep 14, 2016

Copy link
Copy Markdown

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

Mike Richmond (mirichmo) commented Sep 14, 2016

Copy link
Copy Markdown
Member

LGTM.

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

@jeffbi

jeffbi commented Sep 14, 2016 •
edited
Loading

Copy link
Copy Markdown
Author

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

mirichmo
Mike Richmond (mirichmo) suggested changes Sep 15, 2016
View reviewed changes

@mirichmo Mike Richmond (mirichmo) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking this PR pending resolution of the CLA issue

@msftclas

Microsoft Contribution License Agreements (msftclas) commented Sep 21, 2016

Copy link
Copy Markdown

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;

@mirichmo
Mike Richmond (mirichmo) merged commit 5c88dbf into PowerShell:master Sep 22, 2016
@jeffbi
jeffbi deleted the convertto-html-coreclr branch September 22, 2016 22:53
@mirichmo Mike Richmond (mirichmo) mentioned this pull request Jul 17, 2017
Merged
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@mirichmo
Merge pull request PowerShell#2241 from jeffbi/convertto-html-coreclr
18fe81d 
Make ConvertTo-Html work on CoreCLR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@mirichmo Mike Richmond (mirichmo) mirichmo approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@mirichmo Mike Richmond (mirichmo)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jeffbi @msftclas @vors @mirichmo @daxian-dbw @TravisEz13