gaelcolas / Sampler

Module template with build pipeline and examples, including DSC elements.
MIT License
172 stars 42 forks source link

Add ModuleBuilder pattern code coverage support #263

Closed johlju closed 3 years ago

johlju commented 3 years ago

Pull Request

Pull Request (PR) description

Added

Task list


This change is Reviewable

johlju commented 3 years ago

UPDATE: Fixed in a recent commit.

There is a problem with Update-JaCoCoStatistic that I haven't dug into yet. It doesn't update the statistics correctly. I just want to see if this will upload to Codecov.io at all. 🙂 Might be that Codecov.io does not use the statistics.

johlju commented 3 years ago

UPDATE: Fixed in a recent commit.

It seems that Merge-JaCoCoReport doesn't do right against the DTD, so it fails check.

For example it must add nodes in the correct order, must add the report element's counters after the last package element.

<report name="Pester (04/03/2021 18:20:03)">
  <sessioninfo id="this" start="1617473772414" dump="1617474003894" />
  <package name="0.110.0">
  ...
  </package>
  <counter type="INSTRUCTION" missed="481" covered="65" />
  <counter type="LINE" missed="373" covered="58" />
  <counter type="METHOD" missed="21" covered="3" />
  <counter type="CLASS" missed="1" covered="0" />
</report>

Now it looks like

<report name="Pester (04/03/2021 18:20:03)">
  <sessioninfo id="this" start="1617473772414" dump="1617474003894" />
  <counter type="INSTRUCTION" missed="481" covered="65" />
  <counter type="LINE" missed="373" covered="58" />
  <counter type="METHOD" missed="21" covered="3" />
  <counter type="CLASS" missed="1" covered="0" />
  <package name="0.110.0">
  ...
  </package>
</report>
johlju commented 3 years ago

UPDATE: Fixed in a recent commit.

I see I have to convert backslash to forward slash as well.

johlju commented 3 years ago

UPDATE: Fixed in a recent commit.

There seems there is a bug in Merge-JaCoCoReport as well. For each package that does not already exist it adds the entire merge document, which means there are many duplicate package elements.

johlju commented 3 years ago

Testing the upload to Codecov.io here and it seem to work: https://github.com/dsccommunity/DscResource.DocGenerator/pull/71#issuecomment-813068743 Codecov.io is updating the PR as expected.

Would be great to merge this PR so I can continue testing. It should not affect any repo since one have to add the new task to build.yaml.

@ykuijs can you review the changes to *JaCoCo* functions so they does not affect SharePointDsc in a negative way?

johlju commented 3 years ago

@gaelcolas if it would be possible I like to merge this build task and then come back later and split it up into functions to reduce the length of the function/task in another PR.

gaelcolas commented 3 years ago

Yeah sure. let me have a look tomorrow (tentatively).

johlju commented 3 years ago

Great!

Added one more commit - I saw it failed on Windows PowerShell due to it handles (not) loading of DTD differently (looks like more of a bug in [System.Xml.XmlDocument]). This does not happen in PowerShell.

johlju commented 3 years ago

I added a few tiny fixes to comments.

codecov[bot] commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@7efb300). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #263   +/-   ##
=======================================
  Coverage          ?    12%           
=======================================
  Files             ?     23           
  Lines             ?    425           
  Branches          ?      0           
=======================================
  Hits              ?     52           
  Misses            ?    373           
  Partials          ?      0