Subject: Re: [PATCH v2 2/3] t/*: avoid "whitelist"

From: Derrick Stolee <[email-masked]>
Date: Tue, 19 Jul 2022 11:09:35 -0400

On 7/15/2022 7:02 AM, Ævar Arnfjörð Bjarmason wrote:
>>  GIT_TEST_PASSING_SANITIZE_LEAK=<boolean> when compiled with
>> -SANITIZE=leak will run only those tests that have whitelisted
>> -themselves as passing with no memory leaks. Tests can be whitelisted
>> -by setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing
>> -"test-lib.sh" itself at the top of the test script. This test mode is
>> -used by the "linux-leaks" CI target.
>> +SANITIZE=leak will run only those tests that have marked themselves as
>> +passing with no memory leaks by setting "TEST_PASSES_SANITIZE_LEAK=true"
>> +before sourcing "test-lib.sh" itself at the top of the test script. This
>> +test mode is used by the "linux-leaks" CI target.
> 
> It's hard to improve your own verbage, but I think in this case my
> original version can be improved still:
> 
> 	GIT_TEST_PASSING_SANITIZE_LEAK=<bool> when compiled with
> 	SANITIZE=leak will, when true, only run those tests that declare
> 	themselves leak-free by setting "TEST_PASSES_SANITIZE_LEAK=true"
> 	before sourcing "test-lib.sh". This test mode is used by the
> 	"linux-leaks" CI target.

Another iteration:

  GIT_TEST_PASSING_SANITIZE_LEAK=<bool> focuses the test suite on finding
  memory leaks. When the variable is true and Git is compiled with
  SANITIZE=leak, only run those tests that declare themselves leak-free by
  setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing "test-lib.sh".
  This test mode is used by the "linux-leaks" CI target.
 
>> -test_expect_success 'curl redirects respect whitelist' '
>> +test_expect_success 'curl redirects respect allowed protocols' '
> 
> Isn't the real problem here that this is inaccurate with regards to
> "curl", i.e. AFAIK from browsing transport.c the whitelist of protocols
> has nothing to do with curl, we parse that out and apply it before we
> ever get to the specific transport layer.
> 
> So this should just be "http(s) transport respects GIT_ALLOW_PROTOCOL",
> no?

Sounds good.

>> -test_description='test protocol whitelisting with submodules'
>> +test_description='test protocol restrictions with submodules'
> 
> Minor: I think this shows the awkwardness of using a word derived from
> "allow". Before we could use "whitelist" and "whitelisting"
> consistentlry, but now you have "allowed", "allowlist", "restrictions"
> etc.
> 
> I guess you could say "test protocol allowances..." or something? Meh.

Perhaps "filtering" is the best way to describe the higher-level
feature that these lists help to implement.

Thanks,
-Stolee