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

#193 Duplicate part in the pester test is removed. #197

Conversation

ArianArt
Copy link
Contributor

@ArianArt ArianArt commented Jul 2, 2024

No description provided.

@ArianArt ArianArt requested a review from OlegTokar July 2, 2024 13:27
@@ -67,7 +67,7 @@ namespace Trisoft.ISHRemote.Cmdlets.BackgroundTask
/// <code>
/// New-IshSession -WsBaseUrl "https://example.com/ISHWS/"
/// $ishBackgroundTask = Get-IshDocumentObj -LogicalId "GUID-ABCD-1234" |
/// Add-IshBackgroundTask -EventType "SYNCHRONIZEMETRICS" -InputDataTemplate IshObjectWithIshRef
/// Add-IshBackgroundTask -EventType "SYNCHRONIZEMETRICS" -InputDataTemplate IshObjectWithLngRef
Copy link
Contributor

Choose a reason for hiding this comment

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

For example Get-IshDocumentObj -LogicalId ISHEDITORTEMPLATEMODULECONCEPT will return multiple versions and multiple languages. So the description is wrong mentioned "latest". I did not check outside of this PR, but the typical case for SYNCHRONIZEMETRICS should be -InputDataTemplate IshObjectsWithIshRef where the server expands the LogicalId, right?

(Get-IshTypeFieldDefinition -IshSession $ishSession | Where-Object -Property ISHType -EQ 'ISHEvent' | Where-Object -Property IsSystem -EQ $true).Count | Should -Be 23 # all columns are system columns
(Get-IshTypeFieldDefinition -IshSession $ishSession | Where-Object -Property ISHType -EQ 'ISHEvent' | Where-Object -Property Name -EQ 'USERID').DataSource | Should -Be 'ISHUser'
}
(Get-IshTypeFieldDefinition -IshSession $ishSession | Where-Object -Property ISHType -EQ 'ISHEvent' | Where-Object -Property Level -EQ 'Progress').Count | Should -Be 11
Copy link
Contributor

Choose a reason for hiding this comment

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

It is okay, but I don't like updating tests that actually don't belong to story #193 ... you better submit them separately under #4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this one is more related to the issue #191

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed so better to submit is as PR for #191 as that would increase traceability a lot. It is okay to leave for now, but clustering per story helps in the long run...

@ArianArt ArianArt requested a review from ddemeyer July 3, 2024 06:14
@ddemeyer ddemeyer merged commit 0822e9c into master Jul 3, 2024
1 check passed
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.

3 participants