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 — Make ConvertTo-Html work on CoreCLR by jeffbi · Pull Request #2241 · PowerShell/PowerShell

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
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;
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-required label Sep 12, 2016
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

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 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@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>}
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vors sergei (vors) added the Review - Waiting on Author label Sep 12, 2016
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

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.

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

Sorry, something went wrong.

Uh oh!

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

@jeffbi jeffbi Sep 14, 2016

Copy link
Copy Markdown
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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@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;
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 cla-signed and removed cla-required labels Sep 14, 2016
@TravisEz13 Travis Plunk (TravisEz13) added Review - Needed and removed Review - Waiting on Author labels Sep 14, 2016
Jeff Bienstadt
Update test per code review
63bb074
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mirichmo Mike Richmond (mirichmo) assigned Mike Richmond (mirichmo) and unassigned sergei (vors) Sep 14, 2016
@jeffbi

jeffbi commented Sep 14, 2016 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mirichmo
Mike Richmond (mirichmo) suggested changes Sep 15, 2016
View reviewed changes

@mirichmo Mike Richmond (mirichmo) left a comment

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.

Blocking this PR pending resolution of the CLA issue

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mirichmo
Mike Richmond (mirichmo) approved these changes Sep 21, 2016
View reviewed changes
@mirichmo Mike Richmond (mirichmo) closed this Sep 21, 2016
@mirichmo Mike Richmond (mirichmo) reopened this Sep 21, 2016
@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;
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 Sep 21, 2016
@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
Fix ConvertTo-Html single matched column header bug #4276
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

Uh oh!

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

Assignees

@mirichmo Mike Richmond (mirichmo)

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.

6 participants

@jeffbi @msftclas @vors @mirichmo @daxian-dbw @TravisEz13
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.