Extend run-tests.ps1 by a warning if submodules are not synced #557
Conversation
I'm contributing to ImageSharp since around a week, and I saw 3 people (myself included) stumbling over failing tests due to the submodule for test data being out of sync. With this commit the run-tests.ps1 test script is extended by the following behaviour: - if something fails during the script run, the submodule status is checked. A warning is printed if - any submodule is not initialized or - any submodule is not synchronized - git could not be called to check the submodule status.
Codecov Report
@@ Coverage Diff @@
## master #557 +/- ##
==========================================
+ Coverage 88.69% 88.69% +<.01%
==========================================
Files 840 840
Lines 35300 35300
Branches 2544 2544
==========================================
+ Hits 31308 31310 +2
+ Misses 3245 3243 -2
Partials 747 747
Continue to review full report at Codecov.
|
antonfirsov
left a comment
antonfirsov
left a comment
There was a problem hiding this comment.
@jongleur1983 tested locally by clearing tests\Images\External\, but it does not print the error message for me.
After a short debugging it seems, that CheckSubmoduleStatus() returns 0 despite of git submodule status resulting in string -5a9a88380166a87521d10048f53cda7f5f761d66 tests/Images/External (which contains a -)
run-tests.ps1
Outdated
| $submoduleStatus = CheckSubmoduleStatus | ||
| if ([int]$submoduleStatus -eq 1) { | ||
| # not synced | ||
| Write-Host -ForegroundColor Yellow "Check if submodules are up to date. You can use 'git submodule update' to fix this"; |
There was a problem hiding this comment.
Please use spaces for indentation instead of tabs! (Like the rest of the file.)
There was a problem hiding this comment.
resolved, thanks
1) -contains is for arrays, not for strings, so we use match (and mask the -) 2) if a module is not initialized the status contains the string ((null)) where usually the commit id is placed (the Hash in parantheses)
|
Fixes committed. Sorry for the circumstances - I should have tested it better myself.
and tabs are replaced by spaces of course. There may still be cases missing, but I'm not sure how to test them. |
There was a problem hiding this comment.
@jongleur1983 I pushed a failing test to validate the effect of your changes on appveyor. Looks like the execution is about to succeed despite the failing test. run-tests.ps1 should not return 0 to it's caller, when test execution fails!
|
Unexpected behavior confirmed. |
…dule status' accidently.
|
Everything looks good now. @JimBobSquarePants I hope you don't mind that I'm merging this. |
|
@antonfirsov Whenever you think something is good merge away! 😄 |
1) -contains is for arrays, not for strings, so we use match (and mask the -) 2) if a module is not initialized the status contains the string ((null)) where usually the commit id is placed (the Hash in parantheses)
…tsScript Extend run-tests.ps1 by a warning if submodules are not synced
3 participants
Prerequisites
Description
I'm contributing to ImageSharp since around a week, and I saw 3 people (myself included) stumbling over failing tests due to the submodule for test data being out of sync.
With this commit the run-tests.ps1 test script is extended by the following behaviour:
If something fails during the script run, the submodule status is checked.
A warning is printed if
These warnings contain hints on how to solve that potential issue.