Tests for behaviour like Format-Table -A

[PrzemyslawKlys]

During development of PSWinDocumentation I noticed that my PSWriteWord is delivering "unexpected" results. While the tables delivered by my Add-WordTable were kind of "nice" it was not what was expected in this scenario:

$Value | Format-Table -Autosize 

So I wrote some tests for all those scenarios and now my Add-WordTable is delivering expected results. But then I wanted to export same data to Excel and noticed Export-Excel fails on that as well. So I've ported exact same tests to ImportExcel. Following is how "newest version" behaves.

While I am not sure if you expect it the same way that $variable should always display the same result for user in console as in Excel but certainly I do.

Also keep in mind that tests are failing on my computer:

While the Tests don't need additional 3 files

• PSArray.ps1
• PSObjects.ps1
• PSTables.ps1

But the solution I prepared for this (that will come in next pull request) requires it. I add this in this pull request since it's none breaking change, and the tests are using PSObjects.ps1 function. I've not added those 3 files to ImportExcel.psm1 (I guess you have a reason why you need the files explicitly defined in there).

On a side note the next pull request with a fix is failing on some tests (of yours) and since I don't know if it will be accepted or not don't want to spend more time on this.

On another side not, I don't know what performance impact "support" functions have but maybe it's worth the effort to split some things out of Export-Excel outside so it's easier to read and find out what is really happening. It may be just me but it took me a while to understand what you're doing :-)

[dfinke]

Not sure if your PR is failing the build.

Glad you figured your way around the module. :)

[PrzemyslawKlys]

Ok. Fixed. now my tests are "working" as in failing for your module.

[PrzemyslawKlys]

Now to explain the idea about next commit is that there is PSTables.ps1 which contains couple of functions.

• Format-PSTable - this function is responsible for taking any object and based on what is outside object (for example array or object[]) and what is inside object it converts it to Array of Arrays. which basically makes it easily parsable to table by simply foreach row, foreach table. It's based on 3 support functions.

  • Format PSTableConvertType1
  • Format PSTableConvertType2
  • Format PSTableConvertType3

Why 3? Because while testing I've found there are essentially 3 ways to play with objects (the ones I tested) and so I basically wrote them and based on scenario I reuse them. If it's HashTable or OrderedDictionary I use ConvertType3 and so on. I don't know if that's the best way to handle it but well it works :-)

Then there's the file PSArray.ps1. While not entirely necessary during tests it came out that doing:

$Array = @()
$Array += $Object
$Array += $Object

is much slow then doing it via

$List = New-Object System.Collections.ArrayList
$List.Add($Object1) > $null
$List.Add($Object2) > $null

And also it adds one more benefit...

$List.RemoveAt(1)
$List.Remove($Object1)

Probably couple of others. So I just wrapped those up in support functions for easy use and I use them in functions above. Probably would be fairly easy to add support function that would replace 1st row from such created Array of Arrays with new row.

Now during testing I've actually learnt that pipeline works a bit differently so Format-PSTable contains additional switch -SkipTitle which basically means it shouldn't process it. In normal scenario of passing variable it would start 1st row with titles and then follow up with data. In pipeline scenario on first run you run it without -skiptitle, 2nd time you run it with -skiptitle which just takes "table row".

Again I am nowhere sure I've finalized the list of "possible" objects for those functions but it's quite easy to expand it if there is one. Probably one of such cases are within your tests which fail on my next commit.

[PrzemyslawKlys]

As you will see in those tests things got broken. As I don't know your module that good I am trying to work out why they fail (I guess I removed some critical stuff). While I can spend another day or two trying to figure those out I wanted to ask first whether you want to go that way or I shouldn't waste my time.

[PrzemyslawKlys]

Even worse it fails on my tests too.. however only on Appveyor...

And only regarding conversion of $200 to 200. Funny thing is exact same test that fails here.. works fine on my machine.

It either means you expect "special" culture when converting currency and it will fail on non-English currency system or I'm doing something wrong.

[PrzemyslawKlys]

There is also Format-PSPivotTable function that is able to convert standard HashTable or OrderedHashtable into Custom Object essentially making 1st column to 1st row. So instead of Name, Value you get it in reverse. Could be added with a switch to Export-Excel just in case someone wanted to have that or so.

[PrzemyslawKlys]

Now that I work with it a bit more I think there is a problem with this approach a bit. If you have object that is like

$ADSnapshot.ForestInformation = $(
    Get-ADForest | Select-Object DomainNamingMaster, Domains, ForestMode, Sites
)

and where 'domains' can be more then one element, sites more then one element it will create this in wrong way.. where in my case it moves one domain under 'forestmode'. Which is bad. Will work on it a bit more. Let me know if you have ideas yourself.

[PrzemyslawKlys]

Fixed few things. Had to modify your tests a bit because ScriptProperty and AliasProperty are not the only valid properties. There is also NoteProperty. In case of Get-Process it may not be useful but consider this:

$processes = Get-Process
$propertyNames = $Processes[0].psobject.properties.where( {$_.MemberType -eq 'Property'}).name
Write-Color 'Get Process Types 1' -Color Green
$processes[0].PSObject.Properties.MemberType | Sort-Object -Unique

$processes[0].PSObject.Properties.MemberType.Count
$PropertyType = 'AliasProperty', 'ScriptProperty'
$processes[0].PSObject.Properties.Where( { $PropertyType -notcontains $_.MemberType -and $ExcludeProperty -notcontains $_.Name  } ).Count

$PropertyType = 'AliasProperty', 'ScriptProperty', 'NoteProperty' # But this is wrong
$processes[0].PSObject.Properties.Where( { $PropertyType -notcontains $_.MemberType -and $ExcludeProperty -notcontains $_.Name  } ).Count

$myitems0 = @(
    [pscustomobject]@{name = "Joe"; age = 32; info = "Cat lover"},
    [pscustomobject]@{name = "Sue"; age = 29; info = "Dog lover"},
    [pscustomobject]@{name = "Jason"; age = 42; info = "Food lover"}
)

Write-Color 'Get Process Types 2' -Color Yellow
$myItems0[0].PSObject.Properties.MemberType | Sort-Object -Unique

Write-Color 'Get Process Types 3' -Color Yellow
$Object2 = Get-PSDrive | Where { $_.Provider -like '*Registry*' -or $_.Provider -like '*Environment*' -or $_.Provider -like '*FileSystem*' }
$Object2[0].PSObject.Properties.MemberType | Sort-Object -Unique

Notice how PSCustomObject only has NoteProperty.

[dfinke]

Here are the guidelines for PRs.

• All tests must pass in Appveyor for PR to be considered
• PRs are preferred in small batches, both changes and additions
• Add unit tests for the changes submitted in the PR
• Add unit tests for fixes submitted in the PR
• Add inline help for new features
• Add release notes to the README.md
• Add examples for new features to Examples directory

[PrzemyslawKlys]

What is the right module for tests to work locally?

But it still fails for me locally.

[PrzemyslawKlys]

It seems master version of Export-Excel has a problem of pipeline vs no pipeline. I've expanded tests to cover both scenarios. Results for master version via pipeline:

Without pipeline:

Proposed approach:

There seems to be a bit of performance impact thou. For one test I did old version was 16 seconds. New one was 20 seconds (via pipeline). Without pipeline it was 15 vs 30 seconds. Probably can be improved but maybe not at this moment.

[dfinke]

This PR is getting too large in terms of features and changes. Thanks for the work. Making sure they are backward compatible will be a challenge and impact how/when these will be merged

[PrzemyslawKlys]

Problem is Export-Excel is crucial part of your codebase. If you touch one thing it breaks another and it seems you put a lot of things in here that maybe shouldn’t be there in this form. For example you call Export-Excel even when Process section is not used at all just because somewhere in there it has one functionality that is used. Anyways - There are actually 2 things here. 1 is fix for properly addressing all types of data. The other one is transpose. I dont intented to go beyond that.

[PrzemyslawKlys]

And those features are backwards compatible I believe. Things that worked will continue to work.

[dfinke]

Very aware that the function has grown too large. Need proof that the changes work in the wild.

[PrzemyslawKlys]

@jhoneill @dfinke Can you review this? Right now Export-Excel brings a lot of weird results. I have project where I need this working in consistent manner and I believe this does add support for most of additional data types and at same time doesn't break anything. I've another commit for Export-Excel and Open-ExcelPackage/Close-ExcelPackage too but waiting with this one until this is accepted. If this is a problem I'll just release a fork temporary until you add support for other data types your own way.

[dfinke]

Not until I'm finished working with James's changes. Plus, I have not seen anyone respond in the community to help test these changes.

[jhoneill]

Just to reinforce what @dfinke said trying to keep issues and commits to ONE thing (even if the PR is a small batch of commits) really helps

On Currency. In Export Excel we try to convert text to a number if it parses. so we use Try Parse like this

[Double]::TryParse("£123.45", [System.Globalization.NumberStyles]::Any, [System.Globalization.NumberTrueatInfo]::CurrentInfo, [Ref]$number)
On my machine set for British English, $number comes back as 123.45 because £ is the local currency symbol but
[Double]::TryParse("€123.45", [System.Globalization.NumberStyles]::Any, [System.Globalization.NumberFalsetInfo]::CurrentInfo, [Ref]$number)
Doesn't parse as a number, because € is a just treated as a normal character. Change the machine settings to German and it works the other way around.

The test tried to convert "123.45" ; this works in UK & US English and
In Dutch and German where "." is the thousand separator this looks like "Twelve thousand Three Hundred Forty Five" with the separator in the wrong place. From your name I'm guessing you're using Polish settings - which (like French) use space for the thousand separator. So the test string won't parse.
Fortunately only the test is broken and I have changed the string to
"12345"

On Pipeline / no pipeline. Export-Excel only works with piped input. (A quick look at the process block should tell you that).

If we were designing from scratch I don't think we would have such a complicated Export function; Just export; oh can we just have a title / frozen panes/ filtering/ Headings highlighted / No headings / Named Ranges / Tables / Pivot tables/ Charts / Conditional formatting. Can we export to different bits of the same page ? Can we quickly export a SQL table ? Can we put hyperlinks in ? Can we put formulas in and have them calculate ?
I shouldn't speak for @dfinke but if he had seen how the scope was going to expand when he started work my guess is the design might have been different. But now it is out there (and no-one expected downloads to get past 100K) we're stuck with it.
And I am very painfully aware that if you try to change one thing (e.g. for speed), something you hadn't thought of (like working properly in other languages) stops working and you only find out much later. Trying to write tests which cover those things is a very inexact science.

[PrzemyslawKlys]

Well the way I fixed it it works both ways. Via pipeline and standard way. I've also found why it was working on currency as it was on my computer so I simply fixed tests to treat it differently.

I didn't got any feedback but did you get any feedback on your other questions for "tests"? I guess no. People are lazy and until you give them stuff on plate they won't do anything.

I guess the scenarios they have are simple, I build objects differently and they started to fail heavily. That's why I fixed it to be flexible.

This PR is about 3 things:

• Tests showing you things fail on your original code
• Process block changes (to support both pipeline / parameter approach) along with properly addressing how each object should be displayed in Excel
• Format-TransposeTable which basically is able to convert HashTable to Customobject and back which in turn allows easy -Transpose switch

There is nothing more to it. And it fixes that within single Export-Excel. Nowhere else. Small changes. While there are multiple files added but that's because I like to split my functions into seperate files, smaller functions I can easily reuse. If I would be rewriting Export-Excel most likely I would cut out each option into separate function and just call it from the Export-Excel. So there would be like 10 calls for external functions that would have the logic for it.

[dfinke]

@PrzemyslawKlys

In the time you’ve spent writing reasons, you could have spent it on the issues outlined. James and I do not get paid to do this, and don’t spend our waking hours working on it. We do it in our copious free time.

You’re a guest in this repo, please act accordingly.

[PrzemyslawKlys]

@dfinke I apologize. Must be language barrier as english is not my native language. I didn't mean to be harsh or anything. I was responding to last comment to @jhoneill with explanation why I did it, how I did it and what was changed and why. I didn't meant to offend anyone. Sorry again. Good night.