fix(ci): fix shadow-copy test #3629
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3629 +/- ##
==========================================
+ Coverage 75.86% 77.12% +1.25%
==========================================
Files 470 470
Lines 37301 28430 -8871
==========================================
- Hits 28299 21927 -6372
+ Misses 7071 4574 -2497
+ Partials 1931 1929 -2 ☔ View full report in Codecov by Sentry. |
aaron-kasten
changed the title
BUILD.md
Outdated
| @@ -125,6 +125,7 @@ $ make -C site build | |||
|
|
|||
| This will auto-generate [Markdown](https://en.wikipedia.org/wiki/Markdown) files with documentation for currently supported Kopia CLI subcommands and store them under `site/content/docs/Reference/Command-Line` and then generate the website which is stored in `site/public`. | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
nit; drop this
maybe your editor does this automatically ? Some of my vim plugins used to do this until I configured them to not do auto formatting on existing files.
There was a problem hiding this comment.
@Shrekster I added to trigger a test. I'll remove it.
| @@ -46,6 +48,10 @@ func TestShadowCopy(t *testing.T) { | |||
| } | |||
|
|
|||
| sources := clitestutil.ListSnapshotsAndExpectSuccess(t, e) | |||
|
|
|||
| require.NotEmpty(t, sources) | |||
| require.NotEmpty(t, sources[0].Snapshots) | |||
There was a problem hiding this comment.
good check
The existing code is a bit confusing though... would we get at-least one snapshot if we do not have the permission ?
There was a problem hiding this comment.
Nothing to do here but I find a couple more issues with this test, for example getting any other error besides os.ErrPermission would be suppressed at first and fail later ... not a deal breaker, but I'd prefer to fail fast.
There was a problem hiding this comment.
I probably should've added a comment to clarify what this is doing. An error is guaranteed here since the all-zero GUID is not a valid shadow copy ID. This is purely an "are you an admin?" check designed to match what go-vss is doing to know which version of the test to execute.
Also, yes, a Kopia snapshot is created in both scenarios. The test would've simply panicked previously if that didn't happen.
There was a problem hiding this comment.
I probably should've added a comment to clarify what this is doing. An error is guaranteed here since the all-zero GUID is not a valid shadow copy ID. This is purely an "are you an admin?" check designed to match what go-vss is doing to know which version of the test to execute.
It makes sense to add the comment for clarity, it makes it easier to understand the test.
a Kopia snapshot is created in both scenarios. The test would've simply panicked previously if that didn't happen.
Nevertheless, it makes sense to check explicitly than letting the test panic. It makes it easier to track down test failures.
julio-lopez
changed the title
Fixes #3628