Fix Integration Test Failures in xArchive and General Code Quality improvements - Fixes #457 #472
Conversation
…rchive integration tests
Codecov Report
@@ Coverage Diff @@
## dev #472 +/- ##
====================================
+ Coverage 72% 72% +<1%
====================================
Files 27 27
Lines 4016 4022 +6
Branches 4 4
====================================
+ Hits 2916 2921 +5
- Misses 1096 1097 +1
Partials 4 4 |
There was a problem hiding this comment.
Reviewed 11 of 11 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @PlagueHO)
README.md, line 722 at r2 (raw file):
verbose message parameters
Maybe something like "...verbose messages containing path strings now sorrounds the path with qoutes..."? Users might not know what you mean with "message parameters" I think 🤔
DSCResources/MSFT_xArchive/MSFT_xArchive.psm1, line 1193 at r2 (raw file):
) return $ArchiveEntryName.EndsWith('\') -or $ArchiveEntryName.EndsWith('/')
Looks like a regular expression could simplify this, but since this is backported from PSDscResource, let leave it as-is. 🙂
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2097 at r1 (raw file):
$PSScriptRoot
Minor: Even if it is not actually used (creating files), shouldn't we have $TestDrive here instead? Throughout.
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2196 at r1 (raw file):
creation time
last write time?
There was a problem hiding this comment.
Should be good to review again @johlju! 😁 Thanks again
Reviewable status: 8 of 11 files reviewed, 3 unresolved discussions (waiting on @johlju)
README.md, line 722 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
verbose message parametersMaybe something like "...verbose messages containing path strings now sorrounds the path with qoutes..."? Users might not know what you mean with "message parameters" I think 🤔
Done.
Reworded slightly
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2097 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
$PSScriptRootMinor: Even if it is not actually used (creating files), shouldn't we have
$TestDrivehere instead? Throughout.
Done.
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2196 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
creation timelast write time?
Good catch. Fixed!
There was a problem hiding this comment.
Reviewed 3 of 3 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @PlagueHO)
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2196 at r1 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Good catch. Fixed!
Can't see you changed this? 🙂
There was a problem hiding this comment.
Sorry @johlju - forgot to save the file. Fixed now.
Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion (waiting on @johlju)
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2196 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can't see you changed this? 🙂
Doh! Forgot to save the file.
There was a problem hiding this comment.
Haha happens to me too 😄 Awesome work on this!
Reviewed 1 of 1 files at r4.
Reviewable status:complete! all files reviewed, all discussions resolved
|
@PlagueHO No, you didn't send in a commit on this! 😆 I just hit LGTM, and then looked after the change. 😞 |
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @PlagueHO)
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2192 at r4 (raw file):
creation time
Still not last write time here?
There was a problem hiding this comment.
Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion (waiting on @johlju)
Tests/Unit/MSFT_xArchive.Tests.ps1, line 2192 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
creation timeStill not last write time here?
Sorry @johlju - I was looking and correcting the wrong line! Should be good to go.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r5.
Reviewable status:
Fixes files which are getting triggered for re-encoding after recent checkin (possibly #472)
Pull Request (PR) description
This primarily fixes the broken integration tests for xArchive, but also improves the code to make it more readable and testable.
It also improves tests so that they can run on machines without US date format and addresses the issue with archives that use forward slash.
I also started to include improvements that we've made to many of the other repos (such as .VSCode analyzer settings, .gitattributes, style corrections).
Full details:
This Pull Request (PR) fixes the following issues
Issue #458 also does not seem to be an issue any more although I didn't make any specific change to fix it (I actually suspect this was fixed by using the correct .gitattributes with an inclusion for binaries).
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
@johlju , @mgreenegit, @kwirkykat - could any of you review as this should sort out the blocking issue you mentioned at the community call.
This change is