Adding tests for `Remove-Module` #9276

pougetat · 2019-04-02T21:53:32Z

PR Summary

This PR adds approx 50 tests (+3 seconds when running locally) for the remove-module cmdlet.

If anyone has suggestions for test cases I missed please feel free to comment. Some of the cases listed are quite vague for the moment. My objective is to optimize the number of tests written / branches tested. I will also screenshot some codeCov results once I get around to running it on the codebase.

Tests have been added tests for :

PR Context

This PR is aimed at increasing testing coverage for the module related cmdlets which we would like to refactor down the line.
This issue tracks the refactoring work : #8936

PR Checklist

pougetat · 2019-04-02T22:54:17Z

Not too sure why this is is failing for Remove-Module -Name tests when eveything works locally (running just this test locally and no others):

    BeforeEach {
        Import-Module -Name "$testdrive\Modules\Foo\Foo.psd1" -Force
        Import-Module -Name "$testdrive\Modules\Bar\Bar.psd1" -Force
        Import-Module -Name "$testdrive\Modules\Baz\Baz.psd1" -Force

        (Get-Module -Name "Bar", "Baz", "Foo").Name | Should -BeExactly "Bar", "Baz", "Foo"
    }

With the following error :

Expected exactly @('Bar', 'Baz', 'Foo'), but got @('Bar', 'Baz', 'Foo', 'Foo').
53:         (Get-Module -Name "Bar", "Baz", "Foo").Name | Should -BeExactly "Bar", "Baz", "Foo"

For some reason 'Foo' appears twice.

pougetat · 2019-04-03T07:47:08Z

Fixed the CI tests though I'm still not sure why I had two modules named "Foo" in my environment.
My initial thought was that it could be another Pester test that did not clean up its imported modules thus leaving a Foo module present but perhaps Pester tests are sandboxed in some way and these kinds of race conditions aren't possible.
Thoughts ?

iSazonov · 2019-04-04T05:48:23Z

test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1

+        Import-Module -Name "$testdrive\Modules\Bar\Bar.psd1" -Force
+        Import-Module -Name "$testdrive\Modules\Baz\Baz.psd1" -Force
+
+        (Get-Module -Name "Bar", "Baz", "Foo").Name | Should -BeExactly "Bar", "Baz", "Foo", "Foo"


I am not sure that it does that we expect. I suggest join strings and compare.
So all tests should be refactored.

Also please remove the line - all "Should" should be in "It" blocks.

Addressed the second comment.

In regards to the first comment I can confirm that -BeExactly does indeed do what we expect to, at least in the sense that it makes sure that the list of modules we try to get are indeed present (both lists are equal).

…S Session

iSazonov

LGTM with one minor comment.

test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1

iSazonov · 2019-04-23T03:24:29Z

@pougetat Thanks for your contribution!

fMichaleczek · 2019-04-23T05:27:53Z

@pougetat great job ! The only missing scenario is when you import module with using scripting.Classes.using.tests.ps1#L504 )

rjmholt · 2019-04-23T05:31:01Z

@fMichaleczek Please feel free to contribute more tests

fMichaleczek · 2019-04-23T06:59:40Z

@rjmholt sure ! if i write a test that failed (bug in powershell), what I should do with this test ?

rjmholt · 2019-04-23T07:02:04Z

The ideal I think is to open a PR with the test marked as -Pending, so that when the behaviour changes the test passes.

* return correct casing of filesystem path during normalization * handle UNC case * handle case to add trailing separator back * add support for NTFS streams syntax * add case-sensitive test * only apply correct casing to directories * handle 8.3 path syntax * fix short path processing * Remove elements which do not work on earlier version on Windows PowerShell (#9411) * Use `Environment.NewLine` for new lines in `ConsoleHost` code (#9392) * Run CodeFormatter for Eventing (#9394) * Run CodeFormatter for MarkdownRender (#9398) * Run CodeFormatter for Security module (#9399) * Run CodeFormatter for WSMan.Runtime (#9401) * Run CodeFormatter for WSMan.Management (#9400) * Allow CI to run on branches with this name pattern: feature* (#9415) * Run CodeFormatter with BraceNewLine,UsingLocation,FormatDocument,NewLineAbove rules (#9393) * Build(deps): Bump NJsonSchema from 9.13.30 to 9.13.34 (#9421) * Build(deps): Bump NJsonSchema from 9.13.34 to 9.13.35 (#9429) Bumps [NJsonSchema](https://github.com/rsuter/NJsonSchema) from 9.13.34 to 9.13.35. - [Release notes](https://github.com/rsuter/NJsonSchema/releases) - [Commits](https://github.com/rsuter/NJsonSchema/commits) Signed-off-by: dependabot[bot] <support@dependabot.com> * Build(deps): Bump Newtonsoft.Json (#9431) Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) from 12.0.1 to 12.0.2. - [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases) - [Commits](JamesNK/Newtonsoft.Json@12.0.1...12.0.2) Signed-off-by: dependabot[bot] <support@dependabot.com> * Build(deps): Bump Newtonsoft.Json from 12.0.1 to 12.0.2 (#9434) Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) from 12.0.1 to 12.0.2. - [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases) - [Commits](JamesNK/Newtonsoft.Json@12.0.1...12.0.2) Signed-off-by: dependabot[bot] <support@dependabot.com> * Add Preview assets for msix (#9375) * Adding tests for `Remove-Module` (#9276) * Build(deps): Bump gulp from 4.0.0 to 4.0.1 in /test/common/markdown (#9441) * Move warning message to EndProcessing so it only shows up once (#9385) * Run Codeformatter for SMA (#9402) * Fix the failed test and update 'Publish-TestResults' to make AzDO fail the task when any tests failed (#9457) * Enable building on Kali Linux (#9471) * Build test packages for windows, linux-x64, linux-arm, linux-arm64 and macOS (#9476) * Build(deps): Bump NJsonSchema from 9.13.35 to 9.13.36 (#9478) * Update Release_Process.md Add item for global tool Co-authored-by: Steve Lee <slee@microsoft.com> Co-authored-by: Steve Lee (POWERSHELL) <slee@ntdev.microsoft.com> Co-authored-by: Aditya Patwardhan <adityap@microsoft.com> Co-authored-by: Ilya <darpa@yandex.ru> Co-authored-by: Dongbo Wang <dongbow@microsoft.com> Co-authored-by: null <dependabot[bot]@users.noreply.github.com> Co-authored-by: pougetat <thomas.pouget-abadie@ensimag.grenoble-inp.fr> Co-authored-by: Andrew <anmenaga@microsoft.com>

Adding tests for remove-module

a21fd24

Attempting to fix CI tests

a556958

throwaway774 added 2 commits April 3, 2019 11:46

Adding tests for Remove-Module -ModuleInfo

4ecb512

Rearranging tests

4561f23

iSazonov requested review from SteveL-MSFT, adityapatwardhan and rjmholt April 4, 2019 05:40

iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Apr 4, 2019

Adding remove-module readonly modules tests

b0e42cf

pougetat changed the title ~~Adding tests for remove-module~~ [WIP] Adding tests for remove-module Apr 4, 2019

iSazonov reviewed Apr 4, 2019

View reviewed changes

throwaway774 added 8 commits April 4, 2019 13:01

Addressing PR comment

b4c99c2

Adding Const module removal tests + cleaning up other tests

5b3b0d4

Adding tests to remove-module : module provides PSDrive for current P…

8da1d44

…S Session

Adding remove-module test : module contains nested modules

516ec27

test currently failing

6b31ef2

Fixing tests and adding remove-module circular dependencies test

ff3c386

Adding test for remove-module where modules are required by others

eacde04

Adding remove-module module required by other modules test

adc0f24

pougetat changed the title ~~[WIP] Adding tests for remove-module~~ Adding tests for remove-module Apr 5, 2019

rjmholt requested a review from daxian-dbw April 11, 2019 22:13

rjmholt approved these changes Apr 11, 2019

View reviewed changes

iSazonov approved these changes Apr 12, 2019

View reviewed changes

test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 Show resolved Hide resolved

iSazonov assigned adityapatwardhan Apr 12, 2019

iSazonov added this to the 7.0.0-preview.1 milestone Apr 12, 2019

Addressing PR comment

0f147ad

adityapatwardhan approved these changes Apr 22, 2019

View reviewed changes

adityapatwardhan changed the title ~~Adding tests for remove-module~~ Adding tests for remove-module Apr 22, 2019

adityapatwardhan changed the title ~~Adding tests for remove-module~~ Adding tests for Remove-Module Apr 22, 2019

adityapatwardhan merged commit 0bd9377 into PowerShell:master Apr 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding tests for `Remove-Module` #9276

Adding tests for `Remove-Module` #9276

Uh oh!

pougetat commented Apr 2, 2019 •

edited

Loading

Uh oh!

pougetat commented Apr 2, 2019 •

edited

Loading

Uh oh!

pougetat commented Apr 3, 2019

Uh oh!

iSazonov Apr 4, 2019

Uh oh!

pougetat Apr 4, 2019 •

edited

Loading

Uh oh!

iSazonov left a comment

Uh oh!

Uh oh!

iSazonov commented Apr 23, 2019

Uh oh!

fMichaleczek commented Apr 23, 2019

Uh oh!

rjmholt commented Apr 23, 2019

Uh oh!

fMichaleczek commented Apr 23, 2019

Uh oh!

rjmholt commented Apr 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Adding tests for Remove-Module #9276

Adding tests for Remove-Module #9276

Uh oh!

Conversation

pougetat commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

pougetat commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pougetat commented Apr 3, 2019

Uh oh!

iSazonov Apr 4, 2019

Choose a reason for hiding this comment

Uh oh!

pougetat Apr 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iSazonov commented Apr 23, 2019

Uh oh!

fMichaleczek commented Apr 23, 2019

Uh oh!

rjmholt commented Apr 23, 2019

Uh oh!

fMichaleczek commented Apr 23, 2019

Uh oh!

rjmholt commented Apr 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Adding tests for `Remove-Module` #9276

Adding tests for `Remove-Module` #9276

pougetat commented Apr 2, 2019 •

edited

Loading

pougetat commented Apr 2, 2019 •

edited

Loading

pougetat Apr 4, 2019 •

edited

Loading