Re: [PATCH v2 3/4] commit-graph.c: write non-split graphs as read-only
- From
Jeff King <peff@peff.net>
- Date
- Apr 28, 2020, 03:34 UTC
- Message-ID
- <20200428033438.GA2369457@coredump.intra.peff.net>
- In-Reply-To
- <20200427235935.GA14984@syl.local>
On Mon, Apr 27, 2020 at 05:59:35PM -0600, Taylor Blau wrote:
Show 19 quoted lines
> > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh > > index fb0aae61c3..901eb3ecfb 100755 > > --- a/t/t5318-commit-graph.sh > > +++ b/t/t5318-commit-graph.sh > > @@ -12,6 +12,10 @@ test_expect_success 'setup full repo' ' > > test_oid_init > > ' > > > > +test_expect_success POSIXPERM 'tweak umask for modebit tests' ' > > + umask 022 > > +' > > + > > test_expect_success 'verify graph with no graph file' ' > > cd "$TRASH_DIRECTORY/full" && > > git commit-graph verify > > Looks good to me; this is definitely necessary. For what it's worth, it > passes for me, but my system may not have as restrictive a umask as > others.
If we're just doing this for a single test, perhaps it would be better to set the umask in that test (perhaps even in a subshell to avoid touching other tests). I guess that's a little awkward here because the write and the mode-check happen in separate snippets.
Or going in the opposite direction: if we think that covering the rest of the test suite with a diversity of umasks isn't worthwhile, we could just set "umask" in test-lib.sh. That would solve this problem and any future ones.
I also wondered if it would be simpler to just limit the scope of the test, like so:
diff --git b/t/t5318-commit-graph.sh a/t/t5318-commit-graph.sh index fb0aae61c3..5f1c746ad1 100755 --- b/t/t5318-commit-graph.sh +++ a/t/t5318-commit-graph.sh @@ -98,9 +98,10 @@ test_expect_success 'write graph' ' test_expect_success POSIXPERM 'write graph has correct permissions' ' test_path_is_file $objdir/info/commit-graph && - echo "-r--r--r--" >expect && test_modebits $objdir/info/commit-graph >actual && - test_cmp expect actual + # check only user mode bits, as we do not want to rely on + # test environment umask + grep ^-r-- actual ' graph_git_behavior 'graph exists' full commits/3 commits/1 but maybe there's some value in checking the whole thing. -Peff