Re: [PATCH 7/8] t/perf: add 'GIT_PERF_USE_SCALAR' run option

From: Victoria Dye <vdye@github.com>
Date: Sep 2, 2022, 04:00 UTC
Message-ID: <72f6661d-be02-dbb8-7110-b8de6bac77f6@github.com>
In-Reply-To: <60p9nos4-n576-qqq2-p8n9-969sro247141@tzk.qr>

Johannes Schindelin wrote:

Show 36 quoted lines

> Hi Victoria,
> 
> On Wed, 31 Aug 2022, Victoria Dye via GitGitGadget wrote:
> 
>> From: Victoria Dye <vdye@github.com>
>>
>> Add a 'GIT_PERF_USE_SCALAR' environment variable (and corresponding perf
>> config 'useScalar') to register a repository created with any of:
>>
>> * test_perf_fresh_repo
>> * test_perf_default_repo
>> * test_perf_large_repo
>>
>> as a Scalar enlistment. This is intended to allow a developer to test the
>> impact of Scalar on already-defined performance scenarios.
> 
> Great idea!
> 
>> [...]
>> @@ -130,7 +137,11 @@ test_perf_fresh_repo () {
>>  	"$MODERN_GIT" init -q "$repo" &&
>>  	(
>>  		cd "$repo" &&
>> -		test_perf_do_repo_symlink_config_
>> +		test_perf_do_repo_symlink_config_ &&
>> +		if test_bool_env "$GIT_PERF_USE_SCALAR" false
>> +		then
>> +			"$MODERN_SCALAR" register
> 
> Do we need to unregister anything here? My guess is that no, the "global"
> config we're using in tests is "$TRASH_DIRECTORY/.gitconfig", and the side
> effect of scheduling the maintenance task won't matter in practice. But I
> might have missed something and we may want to have an explicit
> `unregister` step.
> 
> What's your take on this?

As you guessed, a '.gitconfig' is created in the trash directory of each test containing the Scalar registration and I haven't seen any issues arising from the scheduled maintenance, so I don't think an 'unregister' is necessary. However, while verifying that, I noticed that the registration wasn't happening *at all* because 'test_bool_env' is currently being used incorrectly. The fix is straightforward - I'll make sure to correct it in the next version.

Thanks!

Show 7 quoted lines

> 
> Ciao,
> Dscho
> 
>> +		fi
>>  	)
>>  }