Fix minor spelling/logic errors and missing TODOs in the test suite
Marco Ricci

Marco Ricci commited on 2026-06-14 22:46:36
Zeige 8 geänderte Dateien mit 61 Einfügungen und 27 Löschungen.

... ...
@@ -283,6 +283,7 @@ class TestTestKeys:
283 283
         assert base64.standard_b64encode(key.public_key_data) == public_key_b64
284 284
         assert key.public_key_data.startswith(string(key_type_name))
285 285
 
286
+    # TODO(the-13th-letter): Put RSA key mangling into helper method.
286 287
     @Parametrize.TEST_KEYS
287 288
     def test_private_keys_are_consistent_with_public_keys(
288 289
         self,
... ...
@@ -531,7 +532,7 @@ class TestStubbedSSHAgentSocketProperOperations:
531 532
         del agent
532 533
 
533 534
     def test_closed_agents_cannot_be_interacted_with(self) -> None:
534
-        """The agent can be closed repeatedly."""
535
+        """The agent cannot be usefully used after close."""
535 536
         with machinery.StubbedSSHAgentSocket() as agent:
536 537
             pass
537 538
         query_request = (
... ...
@@ -1231,7 +1231,7 @@ class TestTempdir:
1231 1231
     # filenames.
1232 1232
     @hypothesis.given(
1233 1233
         env_var=strategies.sampled_from(["TMPDIR", "TEMP", "TMP"]),
1234
-        suffix=strategies.builds(
1234
+        subdir=strategies.builds(
1235 1235
             operator.add,
1236 1236
             strategies.text(
1237 1237
                 tuple(" 0123456789abcdefghijklmnopqrstuvwxyz"),
... ...
@@ -1245,13 +1245,13 @@ class TestTempdir:
1245 1245
             ),
1246 1246
         ),
1247 1247
     )
1248
-    @hypothesis.example(env_var="", suffix=".").via(
1248
+    @hypothesis.example(env_var="", subdir=".").via(
1249 1249
         "static example; branch coverage (test)"
1250 1250
     )
1251 1251
     def test_get_tempdir(
1252 1252
         self,
1253 1253
         env_var: str,
1254
-        suffix: str,
1254
+        subdir: str,
1255 1255
     ) -> None:
1256 1256
         """[`cli_helpers.get_tempdir`][] returns a temporary directory.
1257 1257
 
... ...
@@ -1290,7 +1290,7 @@ class TestTempdir:
1290 1290
             monkeypatch.delenv("TEMP", raising=False)
1291 1291
             monkeypatch.delenv("TMP", raising=False)
1292 1292
             monkeypatch.setattr(tempfile, "tempdir", None)
1293
-            temp_path = pathlib.Path.cwd() / suffix
1293
+            temp_path = pathlib.Path.cwd() / subdir
1294 1294
             if env_var:
1295 1295
                 monkeypatch.setenv(env_var, os.fsdecode(temp_path))
1296 1296
                 stack.enter_context(make_temporary_directory(temp_path))
... ...
@@ -130,12 +130,18 @@ class TestNotesPrinting:
130 130
                     "expected known stderr contents"
131 131
                 )
132 132
 
133
-    @hypothesis.given(notes=Strategies.notes().filter(str.strip))
133
+    @hypothesis.given(notes=Strategies.notes())
134 134
     def test_service_with_notes_actually_prints_notes(
135 135
         self,
136 136
         notes: str,
137 137
     ) -> None:
138
-        """Service notes are printed, if they exist."""
138
+        """Service notes are printed, if they contain content.
139
+
140
+        (Service notes are trimmed before printing, and thus result in
141
+        output if and only if they are non-empty after trimming leading
142
+        and trailing whitespace.)
143
+
144
+        """
139 145
         hypothesis.assume("Error:" not in notes)
140 146
         self._test(notes, notes_placement=None, placement_args=())
141 147
 
... ...
@@ -404,6 +410,8 @@ class TestNotesEditingValid(TestNotesEditing):
404 410
             notes_unchanged=notes_unchanged,
405 411
         )
406 412
 
413
+    # TODO(the-13th-letter): Investigate behavior with respect to
414
+    # whitespace-only notes and the legacy editor interface.
407 415
     @Parametrize.MODERN_EDITOR_INTERFACE
408 416
     @hypothesis.settings(
409 417
         suppress_health_check=[
... ...
@@ -392,6 +392,7 @@ class TestStoreroom:
392 392
                 == data.VAULT_STOREROOM_CONFIG_DATA
393 393
             )
394 394
 
395
+    # TODO(the-13th-letter): Rewrite as property-based test.
395 396
     @pytest_machinery.correctness_focused
396 397
     def test_decrypt_bucket_item_unknown_version(self) -> None:
397 398
         """Fail on unknown versions of the master keys file."""
... ...
@@ -406,6 +407,7 @@ class TestStoreroom:
406 407
         with pytest.raises(ValueError, match="Cannot handle version 255"):
407 408
             storeroom._decrypt_bucket_item(bucket_item, master_keys)
408 409
 
410
+    # TODO(the-13th-letter): Rewrite as property-based test.
409 411
     @pytest_machinery.correctness_focused
410 412
     @Parametrize.BAD_CONFIG
411 413
     def test_decrypt_bucket_file_bad_json_or_version(
... ...
@@ -430,6 +432,7 @@ class TestStoreroom:
430 432
             with pytest.raises(ValueError, match="Invalid bucket file: "):
431 433
                 list(storeroom._decrypt_bucket_file(p, master_keys))
432 434
 
435
+    # TODO(the-13th-letter): Rewrite as property-based test.
433 436
     @pytest_machinery.correctness_focused
434 437
     @Parametrize.BAD_MASTER_KEYS_DATA
435 438
     @Parametrize.STOREROOM_HANDLER
... ...
@@ -451,6 +454,7 @@ class TestStoreroom:
451 454
             with pytest.raises(RuntimeError, match=err_msg):
452 455
                 handler(format="storeroom")
453 456
 
457
+    # TODO(the-13th-letter): Rewrite as property-based test.
454 458
     @pytest_machinery.correctness_focused
455 459
     @Parametrize.BAD_STOREROOM_CONFIG_DATA
456 460
     @Parametrize.STOREROOM_HANDLER
... ...
@@ -481,6 +485,7 @@ class TestStoreroom:
481 485
             with pytest.raises(RuntimeError, match=error_text):
482 486
                 handler(format="storeroom")
483 487
 
488
+    # TODO(the-13th-letter): Rewrite as property-based test.
484 489
     @pytest_machinery.correctness_focused
485 490
     def test_decrypt_keys_wrong_data_length(self) -> None:
486 491
         """Fail on internal structural data of the wrong size.
... ...
@@ -533,6 +538,7 @@ class TestStoreroom:
533 538
                 ),
534 539
             )
535 540
 
541
+    # TODO(the-13th-letter): Generate key as well.
536 542
     @pytest_machinery.correctness_focused
537 543
     @hypothesis.given(
538 544
         data=strategies.binary(
... ...
@@ -328,6 +328,7 @@ class TestCLIUtilities:
328 328
                 == expected.expanduser().resolve()
329 329
             )
330 330
 
331
+    # TODO(the-13th-letter): Revise, or remove.
331 332
     def test_get_vault_path_without_home(self) -> None:
332 333
         """Fail to look up the vault path without `HOME`."""
333 334
 
... ...
@@ -465,6 +466,7 @@ class TestGenericVaultCLIErrors:
465 466
                 catch_exceptions=False,
466 467
             )
467 468
 
469
+    # TODO(the-13th-letter): Rewrite as property-based test.
468 470
     def test_invalid_format(self) -> None:
469 471
         """Reject invalid vault configuration format names."""
470 472
         result = self._call_cli(["-f", "INVALID", "VAULT_PATH"])
... ...
@@ -346,6 +346,8 @@ class TestSequin:
346 346
         steps: Sequence[tuple[int, int | type[sequin.SequinExhaustedError]]]
347 347
         """"""
348 348
 
349
+        # TODO(the-13th-letter): Add more known sequences. Derive those
350
+        # from sequin.js.
349 351
         @strategies.composite
350 352
         @staticmethod
351 353
         def strategy(draw: strategies.DrawFn) -> TestSequin.GenerationSequence:
... ...
@@ -466,6 +468,8 @@ class TestSequin:
466 468
         steps: Sequence[tuple[int, Sequence[int], Sequence[int]]]
467 469
         """"""
468 470
 
471
+        # TODO(the-13th-letter): Check feasibility of encoding
472
+        # unsatisfiable shifts.
469 473
         @strategies.composite
470 474
         @staticmethod
471 475
         def strategy(draw: strategies.DrawFn) -> TestSequin.ShiftSequence:
... ...
@@ -486,7 +490,8 @@ class TestSequin:
486 490
                     operator.add,
487 491
                     no_op_counts_strategy,
488 492
                     true_counts_strategy,
489
-                ).flatmap(strategies.permutations)
493
+                ).flatmap(strategies.permutations),
494
+                label="sequence of bit shift sizes",
490 495
             )
491 496
             bit_sequence: list[int] = []
492 497
             steps: list[tuple[int, Sequence[int], list[int]]] = []
... ...
@@ -494,7 +499,8 @@ class TestSequin:
494 499
                 shift_result = draw(
495 500
                     strategies.lists(
496 501
                         bits_strategy, min_size=count, max_size=count
497
-                    )
502
+                    ),
503
+                    label=f"shift result #{i}",
498 504
                 )
499 505
                 for step in steps[:i]:
500 506
                     step[2].extend(shift_result)
... ...
@@ -627,10 +627,17 @@ class TestSSHProtocolDatatypes(TestStaticFunctionality):
627 627
         [DECODE_ENCODE]: https://hypothesis.works/articles/canonical-serialization/
628 628
 
629 629
         """
630
+        # Both canonicalization functions are built with slightly
631
+        # different primitives.  However, because SSH string encoding is
632
+        # a bijective encoding, their result should be the same.
630 633
         canonical_functions = [self.canonicalize1, self.canonicalize2]
634
+        # Test encoding equivalence.
631 635
         for canon1 in canonical_functions:
632 636
             for canon2 in canonical_functions:
633 637
                 assert canon1(encoded) == canon2(encoded)
638
+        # Test idempotence.
639
+        for canon1 in canonical_functions:
640
+            for canon2 in canonical_functions:
634 641
                 assert canon1(canon2(encoded)) == canon1(encoded)
635 642
 
636 643
     @Parametrize.UINT32_EXCEPTIONS
... ...
@@ -40,6 +40,20 @@ TWITTER_PHRASE = rb"[ (HN_N:lI&<ro=)3'g9"
40 40
 The standard derived passphrase for the "twitter" service, from
41 41
 <i>vault</i>(1)'s test suite.
42 42
 """
43
+INTERCHANGABLE_PHRASE1 = (
44
+    b"plnlrtfpijpuhqylxbgqiiyipieyxvfsavzgxbbcfusqkozwpngsyejqlmjsytrmd"
45
+)
46
+"""
47
+One of two [interchangable passphrases][INTERCHANGABLE_PASSPHRASES],
48
+based on [Wikipedia's page on PBKDF2 (HMAC collisions)][HMAC_COL].
49
+
50
+[INTERCHANGABLE_PASSPHRASES]: https://the13thletter.info/derivepassphrase/0.x/explanation/faq-vault-interchangable-passphrases/ 'What are "interchangable passphrases" in `vault`, and what does that mean in practice?'
51
+[HMAC_COL]: https://en.wikipedia.org/w/index.php?title=PBKDF2&oldid=1264881215#HMAC_collisions
52
+"""
53
+INTERCHANGABLE_PHRASE2 = b"eBkXQTfuBqp'cTcar&g*"
54
+"""
55
+Same purpose as [`INTERCHANGABLE_PHRASE1`][INTERCHANGABLE_PHRASE1].
56
+"""
43 57
 
44 58
 buffer_types: dict[str, Callable[..., Buffer]] = {
45 59
     "bytes": bytes,
... ...
@@ -79,8 +93,9 @@ class Parametrize(types.SimpleNamespace):
79 93
         [
80 94
             "ñ",
81 95
             "Düsseldorf",
82
-            "liberté, egalité, fraternité",
96
+            "Liberté, égalité, fraternité",
83 97
             "ASCII",
98
+            "こんにちは。",
84 99
             b"D\xc3\xbcsseldorf",
85 100
             bytearray([2, 3, 5, 7, 11, 13]),
86 101
         ],
... ...
@@ -352,22 +367,9 @@ class TestPhraseDependence:
352 367
         service=Strategies.text_strategy(),
353 368
     )
354 369
     @hypothesis.example(
355
-        phrases=[
356
-            (
357
-                b"plnlrtfpijpuhqylxbgqiiyipieyxvfs"
358
-                b"avzgxbbcfusqkozwpngsyejqlmjsytrmd"
359
-            ),
360
-            b"eBkXQTfuBqp'cTcar&g*",
361
-        ],
370
+        phrases=[INTERCHANGABLE_PHRASE1, INTERCHANGABLE_PHRASE2],
362 371
         service="any service name here",
363
-    ).xfail(
364
-        reason=(
365
-            "phrases are interchangable (Wikipedia example:"
366
-            "https://en.wikipedia.org/w/index.php?title=PBKDF2&oldid=1264881215#HMAC_collisions"
367
-            ")"
368
-        ),
369
-        raises=AssertionError,
370
-    )
372
+    ).xfail(reason="phrases are interchangable", raises=AssertionError)
371 373
     def test_mixed(self, phrases: Sequence[bytes], service: str) -> None:
372 374
         """The internal hash is dependent on the master passphrase.
373 375
 
... ...
@@ -625,7 +627,7 @@ class TestConstraintSatisfactionFromUpstream(TestVault):
625 627
         assert generated == b"763252593304946694588866"
626 628
 
627 629
 
628
-class TestConstraintSatisfactionThoroughness(TestVault):
630
+class TestConstraintSatisfactionThoroughness:
629 631
     """Test passphrase derivation with the "vault" scheme: constraint satisfaction."""
630 632
 
631 633
     @hypothesis.given(
... ...
@@ -690,7 +692,7 @@ class TestConstraintSatisfactionThoroughness(TestVault):
690 692
             assert count <= repeat
691 693
 
692 694
 
693
-class TestConstraintSatisfactionHeavyDuty(TestVault):
695
+class TestConstraintSatisfactionHeavyDuty:
694 696
     """Test passphrase derivation with the "vault" scheme: constraint satisfaction."""
695 697
 
696 698
     @hypothesis.given(
... ...
@@ -819,6 +821,8 @@ class TestUtilities(TestVault):
819 821
             )
820 822
         assert v._estimate_sufficient_hash_length(8.0) >= entropy
821 823
 
824
+    # TODO(the-13th-letter): Either fold this version into test_entropy,
825
+    # or remove degenerate cases from Parametrize.ENTROPY_RESULTS.
822 826
     def test_hash_length_estimation(self) -> None:
823 827
         """
824 828
         Estimating the entropy and hash length for degenerate cases works.
825 829