Skip to content

Add initial Hypothesis config property tests#2144

Merged
jelmer merged 2 commits into
jelmer:mainfrom
ctoth:hypothesis-config-properties
Apr 20, 2026
Merged

Add initial Hypothesis config property tests#2144
jelmer merged 2 commits into
jelmer:mainfrom
ctoth:hypothesis-config-properties

Conversation

@ctoth
Copy link
Copy Markdown
Contributor

@ctoth ctoth commented Apr 19, 2026

This adds a small first slice of property-based testing using Hypothesis.

I kept this deliberately small so the dependency, CI behavior, and test style can be reviewed independently from bug fixes or broader property-test expansion.

The goal is to establish a separate property-test shape without changing Dulwich's regular test suite:

  • keeps tests.test_suite unchanged
  • keeps the regular unittest runner
  • keeps OSS-Fuzz/Atheris fuzzing separate and unchanged
  • makes Hypothesis an optional test dependency
  • puts property tests under property_tests, outside the regular test suite
  • runs the new tests in CI as a separate step with a deterministic Hypothesis profile
  • starts with ConfigFile because it is pure, fast, and parser/serializer-oriented

The new config property tests cover two things:

  • arbitrary bytes either parse or fail with an expected config parse error
  • generated canonical config files round-trip through write_to_file() and from_file()

While testing this locally, this already exposed at least one small config round-trip issue. I have kept the fix out of this PR so this first review can stay focused on the Hypothesis setup and initial test shape. I'll send the bug fix separately.

Also included:

  • ignore .hypothesis/
  • update the contributor docs to use the current tests.test_suite entry point
  • document the separate property-test command

@ctoth ctoth requested a review from jelmer as a code owner April 19, 2026 20:10
@jelmer
Copy link
Copy Markdown
Owner

jelmer commented Apr 19, 2026

Thanks for working on this! This is really useful.

I don't think this should be a part of the regular test suite, since it won't behave predictably. We don't want contributors to get failures others won't see.

I'd prefer if this was under a separate directory. We could optionally run it as part of the tests, but only if we can prohibit any randomness.

@ctoth ctoth force-pushed the hypothesis-config-properties branch from 011fbb1 to 7ecdcec Compare April 19, 2026 23:59
@ctoth
Copy link
Copy Markdown
Contributor Author

ctoth commented Apr 19, 2026

Thanks, that makes sense. I updated this to keep the property tests out of tests.test_suite and move them into a separate top-level property_tests package.

The regular coverage suite no longer installs the test extra or runs Hypothesis. CI now has a separate Property-based tests step, and the property-test module loads a deterministic Hypothesis profile by default (derandomize=True). The ci and local-deep profiles are deterministic too.

That should keep these useful for parser/serializer coverage without making the regular contributor test run locally variable.

@jelmer jelmer enabled auto-merge April 20, 2026 20:08
@jelmer jelmer merged commit 7e71916 into jelmer:main Apr 20, 2026
34 checks passed
jelmer added a commit that referenced this pull request Apr 20, 2026
Follow-up to #2144.

The initial Hypothesis config round-trip property exposed a case where
quoted trailing whitespace was not preserved when reading config values
back.

For example, a value like `b" "` is written as a quoted config value,
but parsing discarded the whitespace because `_parse_string` buffered
whitespace even while inside quotes.

This changes `_parse_string` so spaces and tabs inside quotes are
preserved immediately, while trailing whitespace outside quotes is still
ignored.

Also included:

- a focused regression test for round-tripping quoted trailing space and
tab values
- removal of the temporary property-test filter that excluded values
ending in space or tab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants