Convert to use shared AppVeyor module and Markdown/Example Tests - Fixes #68 #77

PlagueHO · 2017-07-05T08:48:44Z

This PR converts the resource module to use the shared AppVeyor.psm1 module from DSCResource.Tests. It also fixes the README.MD markdown errors. Support for codecoverage reporting via CodeCov.io has also been added.

This change is

codecov-io · 2017-07-12T00:00:51Z

Codecov Report

❗ No coverage uploaded for pull request base (dev@8a4cca3). Click here to learn what that means.
The diff coverage is n/a.

@@         Coverage Diff         @@
##             dev   #77   +/-   ##
===================================
  Coverage       ?   68%           
===================================
  Files          ?     6           
  Lines          ?   719           
  Branches       ?     0           
===================================
  Hits           ?   494           
  Misses         ?   225           
  Partials       ?     0

PlagueHO · 2017-07-12T00:04:12Z

Hi @RichardSiddaway - I thought I'd tag you for a review on this one because I noticed you're marked as a repo maintainer now 😁 and I've been hammering the other repo maintainers with review requests pretty hard lately.

Any chance you take a look at this one? It is primarily a clean up job to try and begin to bring xComputerManagement up to standards.

I'm think it is OK that we're at 68% unit test coverage, but I'll try and bring it up to the magic 70% mark over the next few months.

johlju · 2017-07-12T13:03:17Z

appveyor.yml

-        if ($res.FailedCount -gt 0) {
-            throw "$($res.FailedCount) tests failed."
-        }
+          Invoke-AppveyorTestScriptTask `


You need -ExcludeTag @() otherwise they are excluded ant you can't opt-in.

johlju · 2017-07-12T13:05:04Z

I just saw that while browsing. Have not done a review. Hoping @RichardSiddaway have time to the full review. 😄

… into dev

PlagueHO · 2017-07-16T21:43:51Z

@johlju - any chance you could review this one for me if you get a moment?

johlju · 2017-07-17T07:49:11Z

Reviewed 3 of 6 files at r1, 1 of 2 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

.markdownlint.json, line 4 at r3 (raw file):

    "default": true,
    "MD029": {
        "style": "ordered"

This should be "one" now. Changed in PR recently merged into DscResource.Tests.

README.md, line 10 at r3 (raw file):

The **xComputerManagement** module contains the following resources:

* xComputer - allows you to configure a computer by changing its name and

Maybe add links to their respectively header on each of these resources?
You don't have to do this in this PR. Okay as-is.

README.md, line 18 at r3 (raw file):

  local computer.
* xPowerPlan - specifies a power plan to activate.

Missing resource xVirtualMemory here

README.md, line 252 at r3 (raw file):

```powershell
configuration Sample_xComputer_ChangeNameAndWorkGroup

I started to comment on the examples, but realized there was a lot style "errors" so you have not fixed. Those. Do you want to fix these in this PR or in another PR?

Comments from Reviewable

PlagueHO · 2017-07-17T20:38:16Z

@johlju - I've borrowed the branch header from xSQLServer README.MD and I'll convert all resource I maintain over to that for consistency. Hope that is OK? Should be good to review now.

Review status: 4 of 6 files reviewed at latest revision, 5 unresolved discussions.

.markdownlint.json, line 4 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

This should be "one" now. Changed in PR recently merged into DscResource.Tests.

Done.

appveyor.yml, line 24 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

You need -ExcludeTag @() otherwise they are excluded ant you can't opt-in.

Done.

README.md, line 10 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Maybe add links to their respectively header on each of these resources?
You don't have to do this in this PR. Okay as-is.

Normally I'd do this, but as I'm converting this to Wiki/Auto-documentation soon (just doing the clean up in stages) it won't make much sense to do so 😁

README.md, line 18 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Missing resource xVirtualMemory here

Good catch! Done

README.md, line 252 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I started to comment on the examples, but realized there was a lot style "errors" so you have not fixed. Those. Do you want to fix these in this PR or in another PR?

I've added another issue to get this fixed 😁 I'm trying to break the clean up of this resource down into stages. These examples will all go anyway once I move to Wiki/Auto-documentation (very soon).

Comments from Reviewable

johlju · 2017-07-18T09:39:37Z

Yes, of course it's okay 😄 Glad that it could be reused. i will review those other repos as well. 😄

You got tests failing
https://ci.appveyor.com/project/PowerShell/xcomputermanagement/build/1.9.244.0#L35

Reviewed 3 of 3 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

appveyor.yml, line 6 at r4 (raw file):

version: 1.9.{build}.0
install:
    - git clone -b Issue-86 https://github.com/PlagueHO/DscResource.Tests

I don't think this should be here. Point to your fork. :)

Comments from Reviewable

PlagueHO · 2017-07-18T09:54:51Z

Thanks for reviewing @johlju - the tests will continue to fail on this one until this PR (PowerShell/DscResource.Tests#158) goes through. Once it is in I'll trigger AppVeyor again and they should then pass.

Review status: 5 of 6 files reviewed at latest revision, 1 unresolved discussion.

appveyor.yml, line 6 at r4 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I don't think this should be here. Point to your fork. :)

Fixed. I was using this PR/repo to test the Custom PSSA rules.

Comments from Reviewable

johlju · 2017-07-18T10:07:21Z

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

PlagueHO · 2017-07-22T00:33:11Z

I'll ignore the codecov not meeting target of 70% as we're only just introducing this. This has been raised in issue #81.

PlagueHO added 16 commits February 2, 2017 22:14

- Updated AppVeyor.yml to use AppVeyor.psm1 module in DSCResource.Tests.

56c9070

Convert to Start-Tasks to Invoke

ca00ae3

Fixed up Markdown tests

088eacb

Suppress example tests

61a72bd

Suppress example tests

8a50471

Added test opt-in file

97dec78

Fix up merge conflicts

7d78ebd

Fix failing errors

d0a2f6a

Switch of Markdown line length rule

c822dd1

Fixed long line errors in Markdown

2b93175

More fixes to README.MD

0737477

Final fixup to README.MD

987784a

Convert AppVeyor to use PowerShell default DSCREsource.tests

b29e813

Fix up AppVeyor

884053e

Fix failing tests

2629d6a

Minor clean up to README.MD

c96ad00

msftclas added the cla-already-signed label Jul 5, 2017

PlagueHO added 2 commits July 5, 2017 20:49

Added missing READMD.MD entries

1b185c0

Fix merge conflicts

69f3b58

PlagueHO added the needs review The pull request needs a code review. label Jul 11, 2017

Added CodeCov.io badges

6441f31

PlagueHO requested a review from RichardSiddaway July 11, 2017 23:59

johlju reviewed Jul 12, 2017

View reviewed changes

PlagueHO added 4 commits July 13, 2017 07:35

Changes as per PR comments

6a3382c

Fixed merge conflicts

afa8e48

Fixed README.MD after fixing merge conflict

a11b744

Merge branch 'dev' of https://github.com/PowerShell/xComputerManagement…

0a53790

… into dev

Corrected README.MD after previous change

afd3917

Clean up and correct badge

e65f65c

Fix up after PR comments

23f8cee

PlagueHO mentioned this pull request Jul 17, 2017

Corrected default value of Ensure in help #66

Closed

Try new PSSA Custom rules

0f06065

Revert to using standard DSCResource.tests

1861413

PlagueHO closed this Jul 22, 2017

PlagueHO reopened this Jul 22, 2017

msftclas added the cla-already-signed label Jul 22, 2017

PlagueHO merged commit 731f23c into dsccommunity:dev Jul 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert to use shared AppVeyor module and Markdown/Example Tests - Fixes #68 #77

Convert to use shared AppVeyor module and Markdown/Example Tests - Fixes #68 #77

PlagueHO commented Jul 5, 2017 •

edited by joeyaiello

Loading

codecov-io commented Jul 12, 2017 •

edited

Loading

PlagueHO commented Jul 12, 2017

johlju Jul 12, 2017

johlju commented Jul 12, 2017

PlagueHO commented Jul 16, 2017

johlju commented Jul 17, 2017

PlagueHO commented Jul 17, 2017

johlju commented Jul 18, 2017

PlagueHO commented Jul 18, 2017

johlju commented Jul 18, 2017

PlagueHO commented Jul 22, 2017

Convert to use shared AppVeyor module and Markdown/Example Tests - Fixes #68 #77

Convert to use shared AppVeyor module and Markdown/Example Tests - Fixes #68 #77

Conversation

PlagueHO commented Jul 5, 2017 • edited by joeyaiello Loading

codecov-io commented Jul 12, 2017 • edited Loading

Codecov Report

PlagueHO commented Jul 12, 2017

johlju Jul 12, 2017

Choose a reason for hiding this comment

johlju commented Jul 12, 2017

PlagueHO commented Jul 16, 2017

johlju commented Jul 17, 2017

PlagueHO commented Jul 17, 2017

johlju commented Jul 18, 2017

PlagueHO commented Jul 18, 2017

johlju commented Jul 18, 2017

PlagueHO commented Jul 22, 2017

PlagueHO commented Jul 5, 2017 •

edited by joeyaiello

Loading

codecov-io commented Jul 12, 2017 •

edited

Loading