Re: [PATCH 3/3] scalar: only warn when background maintenance fails
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jan 27, 2023, 22:14 UTC
- Message-ID
- <7070bd26-3020-5a17-2f51-2df9ae00df8c@github.com>
- In-Reply-To
- <65b1735b-7ea6-5f7c-e1d9-6c986c7beb1d@github.com>
On 1/27/2023 5:06 PM, Victoria Dye wrote:
Show 8 quoted lines
> Derrick Stolee via GitGitGadget wrote:
>> if (toggle_maintenance(1))
>> - return error(_("could not turn on maintenance"));
>> + warning(_("could not turn on maintenance"));
>
> Should we do the same thing for 'unregister_dir()'? Unlike 'register_dir()',
> it doesn't break immediately (and finishes removing the enlistment), but it
> still returns a nonzero error code from 'scalar unregister'.The interesting thing about unregister_dir() is that toggle_maintenance(0) "turns of maintenance" by removing the maintenance.repo config value pointing to this repository, not by removing the maintenance schedule. Thus, we don't get the failure in the same way.
Show 10 quoted lines
>> -test_expect_success 'scalar clone fails when background maintenance fails' ' >> +test_expect_success 'scalar clone warns when background maintenance fails' ' >> GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \ >> - test_must_fail scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && >> + scalar clone "file://$(pwd)/to-clone" maint-fail 2>err && >> grep "could not turn on maintenance" err >> ' > > Similarly, it might be nice to show how 'scalar unregister' behaves when > maintenance fails in the tests.
And the way I found out was by making the same tests, but since they actually never failed or listed a warning (for this reason) I left it out.
Thanks, -Stolee