Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConvertTo-DbaDataTable handle Dataplat.Dbatools.Utility.DbaDateTime as an array #9361

Merged
merged 5 commits into from
May 22, 2024

Conversation

jpomfret
Copy link
Collaborator

Type of Change

  • Bug fix (non-breaking change, fixes test-dbalastbackup dates of backup files outputs Dataplat.Dbatools.Utility.DbaDateTime[]  #9337
  • New feature (non-breaking change, adds functionality, fixes # )
  • Breaking change (affects multiple commands or functionality, fixes # )
  • Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
  • Adding code coverage to existing functionality
  • Pester test is included
  • If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
  • Unit test is included
  • Documentation
  • Build system

Commands to test

Import-Module ./dbatools.psd1 -force
$cred = Get-Credential sqladmin
$inst = Connect-DbaInstance -SqlInstance localhost -SqlCredential $cred
$Results = Test-DbaLastBackup -SqlInstance $inst -Destination $inst -EnableException 
$test = ConvertTo-DbaDataTable -InputObject $Results

@wsmelton
Copy link
Member

Fascinated how this test is passing because this test fails as I expect it to on my local machine
image

$innedobj = New-Object -TypeName psobject -Property @{
        Mission = 'Keep Hank alive'
    }

    Add-Member -Force -InputObject $obj -MemberType NoteProperty -Name myobject -Value $innedobj

The above sets myObject to a value (note that name should be camelCasing 😜) so this test is either false or we shouldn't be setting it to a value at all and just creating the property?

@wsmelton
Copy link
Member

wsmelton commented May 20, 2024

Just a few notes on this test as a whole, not something you must change unless you just want to include it in this PR...

  • checking if a column name is on the object should use -Contains in Pester
image
  • If you want to check if a given property is of a particular type use -BeOfType
image

_The last item is also being done twice in this test for some reason, contexts Property: dbadatetimeArray and Property: myObject are both testing that object. Maybe remove one of them if it is really duplicate?

All of this just makes the test a bit easier to read. A version with the above changes is attached if you want to compare it all in VS Code more easily:

$CommandName = $MyInvocation.MyCommand.Name.Replace(".Tests.ps1", "")
Write-Host -Object "Running $PSCommandPath" -ForegroundColor Cyan
. "$PSScriptRoot\constants.ps1"

Describe "$CommandName Unit Tests" -Tag 'UnitTests' {
    Context "Validate parameters" {
        [object[]]$params = (Get-Command $CommandName).Parameters.Keys | Where-Object {$_ -notin ('WhatIf', 'Confirm')}
        [object[]]$knownParameters = 'InputObject', 'TimeSpanType', 'SizeType', 'IgnoreNull', 'Raw', 'EnableException'
        $knownParameters += [System.Management.Automation.PSCmdlet]::CommonParameters
        It "Should only contain our specific parameters" {
            (@(Compare-Object -ReferenceObject ($knownParameters | Where-Object {$_}) -DifferenceObject $params).Count ) | Should Be 0
        }
    }
}

Describe "Testing data table output when using a complex object" {
    $obj = New-Object -TypeName psobject -Property @{
        guid             = [system.guid]'32ccd4c4-282a-4c0d-997c-7b5deb97f9e0'
        timespan         = New-TimeSpan -Start 2016-10-30 -End 2017-04-30
        datetime         = Get-Date -Year 2016 -Month 10 -Day 30 -Hour 5 -Minute 52 -Second 0 -Millisecond 0
        char             = [System.Char]'T'
        true             = $true
        false            = $false
        null             = [bool]$null
        string           = "it's a boy."
        UInt64           = [System.UInt64]123456
        dbadatetime      = [dbadatetime[]]$(Get-Date -Year 2024 -Month 05 -Day 19 -Hour 5 -Minute 52 -Second 0 -Millisecond 0)
        dbadatetimeArray = [dbadatetime[]]($(Get-Date -Year 2024 -Month 05 -Day 19 -Hour 5 -Minute 52 -Second 0 -Millisecond 0), $(Get-Date -Year 2024 -Month 05 -Day 19 -Hour 5 -Minute 52 -Second 0 -Millisecond 0).AddHours(1))
    }

    $innedobj = New-Object -TypeName psobject -Property @{
        Mission = 'Keep Hank alive'
    }

    Add-Member -Force -InputObject $obj -MemberType NoteProperty -Name myObject -Value $innedobj
    $result = ConvertTo-DbaDataTable -InputObject $obj

    Context "Property: guid" {
        It 'Has a column called "guid"' {
            $result.Columns.ColumnName | Should -Contain 'guid'
        }
        It 'Has a [guid] data type on the column "guid"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'guid' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'guid'
        }
        It 'Has the following guid: "32ccd4c4-282a-4c0d-997c-7b5deb97f9e0"' {
            $result.guid | Should Be '32ccd4c4-282a-4c0d-997c-7b5deb97f9e0'
        }
    }

    Context "Property: timespan" {
        It 'Has a column called "timespan"' {
            $result.Columns.ColumnName | Should -Contain 'timespan'
        }
        It 'Has a [long] data type on the column "timespan"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'timespan' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'Int64'
        }
        It "Has the following timespan: 15724800000" {
            $result.timespan | Should Be 15724800000
        }
    }

    Context "Property: datetime" {
        It 'Has a column called "datetime"' {
            $result.Columns.ColumnName| Should -Contain 'datetime'
        }
        It 'Has a [datetime] data type on the column "datetime"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'datetime' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'datetime'
        }
        It "Has the following datetime: 2016-10-30 05:52:00.000" {
            $date = Get-Date -Year 2016 -Month 10 -Day 30 -Hour 5 -Minute 52 -Second 0 -Millisecond 0
            $result.datetime -eq $date | Should Be $true
        }
    }

    Context "Property: char" {
        It 'Has a column called "char"' {
            $result.Columns.ColumnName | Should -Contain 'char'
        }
        It 'Has a [char] data type on the column "char"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'char' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'char'
        }
        It "Has the following char: T" {
            $result.char | Should Be "T"
        }
    }

    Context "Property: true" {
        It 'Has a column called "true"' {
            $result.Columns.ColumnName | Should -Contain 'true'
        }
        It 'Has a [bool] data type on the column "true"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'true' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'boolean'
        }
        It "Has the following bool: true" {
            $result.true | Should Be $true
        }
    }

    Context "Property: false" {
        It 'Has a column called "false"' {
            $result.Columns.ColumnName | Should -Contain 'false'
        }
        It 'Has a [bool] data type on the column "false"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'false' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'boolean'
        }
        It "Has the following bool: false" {
            $result.false | Should Be $false
        }
    }

    Context "Property: null" {
        It 'Has a column called "null"' {
            $result.Columns.ColumnName | Should -Contain 'null'
        }
        It 'Has a [bool] data type on the column "null"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'null' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'boolean'
        }
        It "Has the following bool: false" {
            $result.null | Should Be $false #should actually be $null but its hard to compare :)
        }
    }

    Context "Property: string" {
        It 'Has a column called "string"' {
            $result.Columns.ColumnName | Should -Contain 'string'
        }
        It 'Has a [string] data type on the column "string"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'string' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'string'
        }
        It "Has the following string: it's a boy." {
            $result.string | Should Be "it's a boy."
        }
    }

    Context "Property: UInt64" {
        It 'Has a column called "UInt64"' {
            $result.Columns.ColumnName | Should -Contain 'UInt64'
        }
        It 'Has a [UInt64] data type on the column "UInt64"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'UInt64' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'UInt64'
        }
        It "Has the following number: 123456" {
            $result.UInt64 | Should Be 123456
        }
    }

    Context "Property: myObject" {
        It 'Has a column called "myObject"' {
            $result.Columns.ColumnName | Should -Contain 'myObject'
        }
        It 'Has a [string] data type on the column "myObject"' {
            $result.myObject | Should -BeOfType [System.String]
        }
        It "Has no value" {
            # not sure if this is a feature. Should probably be changed in the future
            $result.myObject.GetType().FullName | Should Be "System.DBNull"
        }
    }

    Context "Property: dbadatetime" {
        It 'Has a column called "dbadatetime"' {
            $result.Columns.ColumnName | Should -Contain 'dbadatetime'
        }
        It 'Has a [dbadatetime] data type on the column "myObject"' {
            $result.Columns | Where-Object -Property 'ColumnName' -eq 'dbadatetime' | Select-Object -ExpandProperty 'DataType' | Select-Object -ExpandProperty Name | Should Be 'string'
        }
        It "Has the following dbadatetime: 2024-05-19 05:52:00.000" {
            $date = Get-Date -Year 2024 -Month 5 -Day 19 -Hour 5 -Minute 52 -Second 0 -Millisecond 0
            [datetime]$result.dbadatetime -eq $date | Should Be $true
        }
    }

    Context "Property: dbadatetimeArray" {
        It 'Has a column called "myObject"' {
            $result.Columns.ColumnName | Should -Contain 'myObject'
        }
        It 'Has a [string] data type on the column "myObject"' {
            $result.myObject | Should -BeOfType [System.String]
        }
        It "Has the following dbadatetimes converted to strings: 2024-05-19 05:52:00.000, 2024-05-19 06:52:00.000" {
            $string = '2024-05-19 05:52:00.000, 2024-05-19 06:52:00.000'
            $result.dbadatetimeArray -eq $string | Should Be $true
        }
    }

}

Describe "Testing input parameters" {
    $obj = New-Object -TypeName psobject -Property @{
        timespan = New-TimeSpan -Start 2017-01-01 -End 2017-01-02
    }

    Context "Verifying TimeSpanType" {
        It "Should return '1.00:00:00' when String is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType String).Timespan | Should Be '1.00:00:00'
        }
        It "Should return 864000000000 when Ticks is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType Ticks).Timespan | Should Be 864000000000
        }
        It "Should return 1 when TotalDays is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType TotalDays).Timespan | Should Be 1
        }
        It "Should return 24 when TotalHours is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType TotalHours).Timespan | Should Be 24
        }
        It "Should return 86400000 when TotalMilliseconds is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType TotalMilliseconds).Timespan | Should Be 86400000
        }
        It "Should return 1440 when TotalMinutes is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType TotalMinutes).Timespan | Should Be 1440
        }
        It "Should return 86400 when TotalSeconds is used" {
            (ConvertTo-DbaDataTable -InputObject $obj -TimeSpanType TotalSeconds).Timespan | Should Be 86400
        }
    }

    Context "Verifying IgnoreNull" {
        # To be able to force null
        function returnnull {
            [CmdletBinding()]
            param ()
            New-Object -TypeName psobject -Property @{ Name = [int]1 }
            $null
            New-Object -TypeName psobject -Property @{ Name = [int]3 }
        }

        function returnOnlynull {
            [CmdletBinding()]
            param ()
            $null
        }

        It "Does not create row if null is in array when IgnoreNull is set" {
            $result = ConvertTo-DbaDataTable -InputObject (returnnull) -IgnoreNull -WarningAction SilentlyContinue
            $result.Rows.Count | Should Be 2
        }

        It "Does not create row if null is in pipeline when IgnoreNull is set" {
            $result = returnnull | ConvertTo-DbaDataTable -IgnoreNull -WarningAction SilentlyContinue
            $result.Rows.Count | Should Be 2
        }

        It "Returns empty row when null value is provided (without IgnoreNull)" {
            $result = ConvertTo-DbaDataTable -InputObject (returnnull)
            $result.Name[0] | Should Be 1
            $result.Name[1].GetType().FullName | Should Be 'System.DBNull'
            $result.Name[2] | Should Be 3
        }

        It "Returns empty row when null value is passed in pipe (without IgnoreNull)" {
            $result = returnnull | ConvertTo-DbaDataTable
            $result.Name[0] | Should Be 1
            $result.Name[1].GetType().FullName | Should Be 'System.DBNull'
            $result.Name[2] | Should Be 3
        }
    }

    Context "Verifying Silent" {
        # To be able to force null
        function returnnull {
            New-Object -TypeName psobject -Property @{ Name = 1 }
            $null
            New-Object -TypeName psobject -Property @{ Name = 3 }
        }

        It "Suppresses warning messages when Silent is used" {
            $null = ConvertTo-DbaDataTable -InputObject (returnnull) -IgnoreNull -EnableException -WarningVariable warn -WarningAction SilentlyContinue
            $warn.message -eq $null | Should Be $true
        }
    }

    Context "Verifying script properties returning null" {

        It "Returns string column if a script property returns null" {
            $myobj = New-Object -TypeName psobject -Property @{ Name = 'Test' }
            $myobj | Add-Member -Force -MemberType ScriptProperty -Name ScriptNothing -Value { $null }
            $r = ConvertTo-DbaDataTable -InputObject $myobj
            ($r.Columns | Where-Object ColumnName -eq ScriptNothing | Select-Object -ExpandProperty DataType).ToString() | Should Be 'System.String'

        }
    }

    Context "Verifying a datatable gets cloned when passed in" {
        $obj = New-Object -TypeName psobject -Property @{
            col1 = 'col1'
            col2 = 'col2'
        }
        $first = $obj | ConvertTo-DbaDataTable
        $second = $first | ConvertTo-DbaDataTable
        It "Should have the same columns" {
            # does not add ugly RowError,RowState Table, ItemArray, HasErrors
            $firstColumns = ($first.Columns.ColumnName | Sort-Object) -Join ','
            $secondColumns = ($second.Columns.ColumnName | Sort-Object) -Join ','
            $firstColumns | Should -Be $secondColumns
        }
    }
}

@wsmelton wsmelton changed the title Fixing #9337 ConvertTo-DbaDataTable handle Dataplat.Dbatools.Utility.DbaDateTime as an array May 20, 2024
@jpomfret
Copy link
Collaborator Author

Fascinated how this test is passing because this test fails as I expect it to on my local machine

Ok I wondered how this was working because it wasn't working for me locally either - I've changed this to make sense (in my mind) and also updated the null one

@potatoqualitee
Copy link
Member

oh sweet, we ready? thank you so much! i cant publish till i get home bc of the hardware key but happy to prep with a merge.

@potatoqualitee potatoqualitee merged commit 3470250 into development May 22, 2024
10 checks passed
@potatoqualitee potatoqualitee deleted the convertTable branch May 22, 2024 10:04
@potatoqualitee
Copy link
Member

re-read and indeed. thanks again 🙇🏼

@potatoqualitee
Copy link
Member

okay that's weird, we have a test that passed but then when it merged, it failed?

Describe : Testing data table output when using a complex object
Context  : Property: myObject
Name     : It Has a [string] data type on the column "myObject"
Result   : Failed
Message  : Expected the value to have type [string] or any of its subtypes, but got  with type [System.DBNull].

@potatoqualitee
Copy link
Member

I skipped it in dev since its causing all of our branches to fail

@jpomfret
Copy link
Collaborator Author

Hey @potatoqualitee - I'm in Lisbon to see Taylor swift tonight 😂 but I'll take a look at this! It did pass locally and on the branch before merge 🤔

@potatoqualitee
Copy link
Member

HEYYY enjoy taylor swift!!! we can look at this later 😊

@jpomfret
Copy link
Collaborator Author

jpomfret commented Jun 2, 2024

@potatoqualitee

I created a new branch off develoment and removed the -skip the tests ran fine 🤔 - also testing locally and it runs fine too.

https://github.com/dataplat/dbatools/tree/testingtests

https://ci.appveyor.com/project/dataplat/dbatools/builds/49936347
image

Is it worth uncommenting in development and seeing what happens? If not we can maybe troubleshoot and fix forward?

Honestly it's kind of a weird test anyway - we already test string values get converted ok - so I'm not sure what this is adding.

    Context "Property: myObject" {
        It 'Has a column called "myObject"' {
            $result.Columns.ColumnName | Should -Contain 'myObject'
        }
        It 'Has a [string] data type on the column "myObject"' {
            $result.myObject | Should -BeOfType [System.String]
        }
    }

@potatoqualitee
Copy link
Member

@jpomfret beautiful! and pfewf. So no change needed to the command, eh? Please do feel free to create a PR and I'll merge with the next batch.

@jpomfret
Copy link
Collaborator Author

jpomfret commented Jun 5, 2024

so @potatoqualitee ... when I went to PR the test was still skipped 🤦

let me try this again - please hold 😂😂

@jpomfret
Copy link
Collaborator Author

jpomfret commented Jun 5, 2024

This is so interesting - this test works completely different in appveyor vs locally

@wsmelton - remember this test - neither of us knew how it was passing in appveyor - I changed it to what was passing locally - and now it's failing in appveyor 😖
#9361 (comment)

image

Shall I remove this test? or change it back to testing for null?

@niphlod
Copy link
Contributor

niphlod commented Jun 5, 2024

lemme test "on my rig" and get back.

@potatoqualitee
Copy link
Member

lol thanks niph, i had no idea how to make it pass

@niphlod
Copy link
Contributor

niphlod commented Jun 5, 2024

yeah. beware that we have another problem, I don't know why "scenario 2008R2, part=1/2" for 2 weeks roughly is sooo borked that it fails badly an the "thingy" that reports back if something fails doesn't know what to read. It seems a pester 5 dependency has slipped in.

https://ci.appveyor.com/project/dataplat/dbatools/builds/49958879/job/0386qjwfw6ofci83 , in particular https://ci.appveyor.com/project/dataplat/dbatools/builds/49958879/job/0386qjwfw6ofci83/tests and
image

I don't know how to fix it but we may try to alter appveyor.yml on development to let it clear and re-cache dependencies, as most of them are linked to the content of appveyor.yml itself .

@niphlod
Copy link
Contributor

niphlod commented Jun 5, 2024

tried my best, AND added some debug prints to uncover the "problem" , see https://ci.appveyor.com/project/dataplat/dbatools/build/job/68qbb1hlj9p98v9w

specifically, I think there might be some "strange happenings" around the fact that:

  • ConvertTo-DbaDataTable returns , $datatable
  • in .NET, a System.Data.DataTable, to access the value of a column named "col1" you need to do dt.Rows[0]['col1']
  • PS may "reduce" the , $datatable, in the test case $result to the first occurrence
  • tried various accessors, the result doesn't change
  • if you try to "dump" via Json, the original object has the proper values, but trying to "dump" the column via Json ends up in a DBNull, indeed
  • dumping the datatable via Json shows
"Columns":  [
                                  "inlining2",
                                  "true",
                                  "char",
                                  "inlining",
                                  "dbadatetimeArray",
                                  "null",
                                  "guid",
                                  "dbadatetime",
                                  "UInt",
                                  "string",
                                  "timespan",
                                  "datetime",
                                  "false",
                                  "myObject"
                              ]
...

"ItemArray":  [
                      "System.Collections.Hashtable",
                      true,
                      "T",
                      null,
                      "2024-05-19 05:52:00.000, 2024-05-19 06:52:00.000",
                      null,
                      "32ccd4c4-282a-4c0d-997c-7b5deb97f9e0",
                      "2024-05-19 05:52:00.000",
                      123456,
                      "it\u0027s a boy.",
                      15724800000,
                      "\/Date(1477806720000)\/",
                      false,
                      null
                  ]

That being said, bumping appveyor.yml did fix the weird issue of a rogue pester 5 in the cache, and I modified:

  • something around manual.pester.ps1 to be more precise
  • appveyor.pester.ps1 now dumps in the log what specific version of pester the test is run with

@jpomfret
Copy link
Collaborator Author

jpomfret commented Jun 6, 2024

Thanks @niphlod for trying - so what do we think for this test, are we thinking it's not really testing what we want? Shall I remove that piece? 🤔

@niphlod
Copy link
Contributor

niphlod commented Jun 6, 2024

pragmatically ..... something is going awry so if it's the one test causing problems (and, btw, it seems unrelated to serializing succesfully dbadatetime[]) so you may choose to just forget about it.
ideally this shows that at least with the combo of software/os/etc on appveyor, it appears that noteproperties are not succesfully serialized as they are "on our rigs".

jpomfret referenced this pull request Jun 7, 2024
/cc @jpomfret

This failed and I'm not sure how I thought it passed in the branch?
@jpomfret
Copy link
Collaborator Author

jpomfret commented Jun 7, 2024

Hmm @niphlod - can you check this failure for me? On the firstRow stuff you added in:
https://ci.appveyor.com/project/dataplat/dbatools/builds/49972082/job/rw3n9xlu79jedh3j

@niphlod
Copy link
Contributor

niphlod commented Jun 7, 2024

sure @jpomfret , lemme revert all the "debug" prints, it was just to illustrate that the test fail(ed) because, indeed, that property was null in the first place.

@niphlod
Copy link
Contributor

niphlod commented Jun 7, 2024

done, it should pass now (and not spit a blob of magenta lines).

@jpomfret
Copy link
Collaborator Author

jpomfret commented Jun 7, 2024

Thank you 👏

@niphlod
Copy link
Contributor

niphlod commented Jun 7, 2024

BTW, to fix this I'll open another PR (end of day, I promise).
It'll include fixes to appveyor.yml, appveyor.pester.ps1 and manual.pester.ps1 so:

  • cache will be pruned (it seems that a pester5 has slipped in)
  • we log what pester version runs the test, so next time it happens it's clearer what the test runner is doing
  • we help users running manual.pester.ps1 notifying that an incorrect version of pester is loaded on their rigs

They were within the changes I pushed on this branch but they got reverted... better keep this PR "on point" and a separate one will make things better for everyone on the testing side.

@niphlod
Copy link
Contributor

niphlod commented Jun 7, 2024

I stand corrected, #9384 has everything in.

@niphlod niphlod mentioned this pull request Jun 7, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test-dbalastbackup dates of backup files outputs Dataplat.Dbatools.Utility.DbaDateTime[]
4 participants