PowerShell / DscResource.Tests

Common meta tests for PowerShell DSC resources repositories.
MIT License
51 stars 49 forks source link

Hashtable check - quick fix for LF end of line #355

Closed SSvilen closed 5 years ago

SSvilen commented 5 years ago

Pull Request (PR) description

This PR extends the Measure-Hashtable function to cover LF end of line.

This Pull Request (PR) fixes the following issues

None

Task list


This change is Reviewable

codecov-io commented 5 years ago

Codecov Report

Merging #355 into dev will decrease coverage by <1%. The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #355   +/-   ##
===================================
- Coverage    84%    84%   -1%     
===================================
  Files        12     12           
  Lines      1750   1750           
  Branches      2      2           
===================================
- Hits       1477   1474    -3     
- Misses      271    274    +3     
  Partials      2      2
PlagueHO commented 5 years ago

Hi @SSvilen, I'm just going to check this against ComputerManagementDsc and NetworkingDsc and see if it resolves the issues. We will also need to copy this change over into https://github.com/dsccommunity/DscResource.AnalyzerRules.

PlagueHO commented 5 years ago

Hi @SSvilen - I'm scratching my head over this one. All hash tables are failing the validation check when run in AppVeyor - but are fine when run locally.

See: https://ci.appveyor.com/project/PowerShell/networkingdsc?fullLog=true#L1293 https://github.com/PowerShell/NetworkingDsc/blob/dev/DSCResources/MSFT_DefaultGatewayAddress/MSFT_DefaultGatewayAddress.psm1#L60

Is it possible that there is an issue with the CRLF? I'm actually wondering if the problem is in this line: https://github.com/PowerShell/DscResource.Tests/blob/dev/DscResource.AnalyzerRules/DscResource.AnalyzerRules.psm1#L1110

The split is using CR rather than CRLF which might be a problem. I've not had time to dig into this further however.

PlagueHO commented 5 years ago

Hi @SSvilen - the comment I made above (see https://github.com/PowerShell/DscResource.Tests/pull/355#issuecomment-552167007) was actually when I tried using your new branch. It doesn't look like it resolves the problem :cry: E.g. I ran NetworkingDsc and ComputerManagementDsc against your branch with the updated RegEx (https://github.com/PowerShell/DscResource.Tests/blob/fc41c12a292fd5a691050ed19fd77d8c404042f8/DscResource.AnalyzerRules/DscResource.AnalyzerRules.psm1#L1113) and it still fails.

So I'm thinking there must be something else going on - which is why I'm wondering about the split.

The reason I query the split, is that on a file with CRLF line endings, that split will result in all of the strings in the array (except entry 0) beginning with a \n - which might mess up the RegEx matches on [1]. I'm just guessing though.

SSvilen commented 5 years ago

That is strange, because I tested it on the AppVeyor server directly.. I'll give it a try tonight again..

SSvilen commented 5 years ago

Hi @PlagueHO,

I actually tested NetworkingDsc against this fix and it passes all appveyor tets. What I changed is:

  1. Fix a cople of hahtable styling violations
  2. In appveyor.yaml I changed the source of DSCResources.Tests to my branch Here are the changes.

The tests results are here.

Is there anything else I should test?

PlagueHO commented 5 years ago

Hi @SSvilen - I definitely am not sure what was going on. But you're right - it appears this has fixed the issue. Sorry about the confusion. Must have too many branches going at the moment!