Skip to content

Test utils feature unification#1599

Merged
benalleng merged 1 commit into
payjoin:masterfrom
benalleng:test-utils-feature-unification
Jun 4, 2026
Merged

Test utils feature unification#1599
benalleng merged 1 commit into
payjoin:masterfrom
benalleng:test-utils-feature-unification

Conversation

@benalleng

Copy link
Copy Markdown
Collaborator

Closes #1598

This allows the payjoin crate to only test the v1 feature gated tests on their own without also pulling in v2 as payjoin-test-utils now no longer automatically pulls in the v2 feature.

Claude caught the directory.rs issue which was occuring because of the v2 only re-export of ShortId.

Pull Request Checklist

Please confirm the following before requesting review:

@benalleng benalleng force-pushed the test-utils-feature-unification branch 2 times, most recently from 7b575f3 to 53314dc Compare June 3, 2026 15:59
@benalleng benalleng force-pushed the test-utils-feature-unification branch from 53314dc to 16b314a Compare June 3, 2026 16:00
@benalleng benalleng marked this pull request as ready for review June 3, 2026 16:00
@benalleng benalleng requested a review from xstoicunicornx June 3, 2026 16:01
@benalleng benalleng removed the blocked label Jun 3, 2026
@coveralls

coveralls commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 26896944251

Coverage remained the same at 85.358%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 8 uncovered changes across 1 file (109 of 117 lines covered, 93.16%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
payjoin-test-utils/src/v2.rs 117 109 93.16%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
payjoin-test-utils/src/lib.rs 2 81.82%

Coverage Stats

Coverage Status
Relevant Lines: 13857
Covered Lines: 11828
Line Coverage: 85.36%
Coverage Strength: 392.2 hits per line

💛 - Coveralls

@bc1cindy bc1cindy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice decoupling

is there a CI job actually testing v1 on its own? without it, v2 can quietly leak back into the v1 path

@benalleng

benalleng commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

is there a CI job actually testing v1 on its own?

Yep, 76f2153 adds fetaure specific tests directly to ./contrib/test.sh. This Pr is a direct followup to ensure that the v1 and v2 tests are properly feature gated.

@bc1cindy bc1cindy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

utACK

@xstoicunicornx xstoicunicornx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK 16b314a

These tests are so organized now nice work!

Reviewed and tested with the various feature flag combos. v1 only runs v1 tests, v2 only runs v2 tests, and only when both flags are active are v1/v2 combined integration tests run. LGTM.

@benalleng benalleng merged commit 895d677 into payjoin:master Jun 4, 2026
14 checks passed
@benalleng benalleng mentioned this pull request Jun 7, 2026
17 tasks
@DanGould

DanGould commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

This breaks publication. Trilemma explained in #1613

DanGould added a commit that referenced this pull request Jun 8, 2026
The public `v2` feature referenced `payjoin-test-utils/v2`, but
payjoin-test-utils is a dev-dependency that itself depends on payjoin.
Referencing a dev-dependency feature from a published feature makes the
manifest unpublishable: cargo resolves the dev-dependency against
crates.io, where no test-utils version exposes `v2`, and none could,
since test-utils depends on this still-unpublished payjoin version.

Drop the test-utils edge from the `v2` feature and declare the
dev-dependency path-only with `v2` enabled directly. Cargo omits
path-only dev-dependencies from the published manifest, so payjoin
packages cleanly. The test-utils helpers are only used under cfg(test),
so the shipped library is unchanged.

This reintroduces v2 into `--features v1` test builds through feature
unification (lib test count rises from 125 to 217), undoing the
isolation added in #1599. Restoring that isolation requires splitting
payjoin-test-utils and is tracked as a follow-up.
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.

Move to drop io by default in payjoin-test-utils

5 participants