Skip to content

Deduplicate identifiers in property tests generators#43

Merged
dancewithheart merged 2 commits into
masterfrom
fix-42-dedup-generated-ids
Jun 9, 2026
Merged

Deduplicate identifiers in property tests generators#43
dancewithheart merged 2 commits into
masterfrom
fix-42-dedup-generated-ids

Conversation

@dancewithheart

@dancewithheart dancewithheart commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Fix #42

Generator could create multiple elements with the same id - ids were created from names and names could have duplicates.

Now names are unique.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR refactors the QuickCheck test generator for SimplePlan by changing how base package names are generated. The genBaseName function is redefined to accept a maximum size and return a deterministic list of sequential package names instead of a single random string, and the generator is updated to use this new function with a fixed bound of 20 packages.

Changes

Deterministic Package Name Generation

Layer / File(s) Summary
Bounded deterministic genBaseName and SimplePlan integration
test/TestSupport.hs
genBaseName function signature changes from Gen String to Int -> Gen [String] and now returns deterministic sequential names (pkg1..pkgN) instead of random strings. SimplePlan generator is updated to call genBaseName 20 for bounded test data.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

A rabbit hops through test support land,
Where generators dance at command,
From strings chaotic to names so neat—
pkg1 through pkgN, deterministic sweet! 🐰
The bounds now stable, the tests repeat. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Deduplicate identifiers in property tests generators' directly aligns with the PR's primary objective of ensuring property test generators produce unique identifiers to prevent duplicates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-42-dedup-generated-ids

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/TestSupport.hs`:
- Around line 54-58: The genBaseName function correctly generates unique names
but uses return; replace the final "return names" in genBaseName with "pure
names" to follow modern Haskell style and applicative-preferred idioms (function
name: genBaseName).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5ad29c8a-f338-4695-9956-2ce66fc4ae74

📥 Commits

Reviewing files that changed from the base of the PR and between 7b19fa9 and 1f455e0.

📒 Files selected for processing (1)
  • test/TestSupport.hs

Comment thread test/TestSupport.hs
@dancewithheart dancewithheart merged commit 0138d21 into master Jun 9, 2026
7 checks passed
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.

Property test fails with seed 2004351319 - duplicated ids for generated dependencies

1 participant