On Mon, Apr 27, 2020 at 05:59:35PM -0600, Taylor Blau wrote: > > 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
