Skip to content

refactor: consolidate TestRepo into single src/testing module#1991

Merged
max-sixty merged 3 commits intomainfrom
testing
Apr 7, 2026
Merged

refactor: consolidate TestRepo into single src/testing module#1991
max-sixty merged 3 commits intomainfrom
testing

Conversation

@max-sixty
Copy link
Copy Markdown
Owner

Two TestRepo structs existed — a lightweight one in src/testing.rs (74 lines, for unit tests) and a full-featured one in tests/common/mod.rs (~1500 lines, for integration tests). This consolidates them into one struct in src/testing/ that serves both.

The key blocker was the integration TestRepo's _snapshot_guard: insta::internals::SettingsBindDropGuard field — insta is a dev-dependency and can't appear in pub code in src/. Replaced with Option<Box<dyn Any>> (type-erased). Snapshot guard setup moved to the repo() rstest fixture.

What moved to src/testing/:

  • TestRepo struct with all ~80 methods
  • BareRepoTest, TestRepoBase trait
  • mock_commands module
  • Helper functions (env isolation, fixture copying, CLI command builders, wait utilities)
  • Constants (TEST_EPOCH, timing constants, etc.)

What stayed in tests/common/:

  • rstest fixtures (repo(), repo_with_*, etc.)
  • Snapshot settings functions (all insta::Settings code)
  • PTY functions (portable_pty code)
  • Re-exports: pub use worktrunk::testing::*

Constructor semantics:

  • TestRepo::new() — lightweight git init + identity (backward-compatible with unit tests)
  • TestRepo::standard() — fixture copy with remote + worktrees + mock gh (what integration new() was)
  • TestRepo::with_initial_commit() — lightweight + one commit
  • TestRepo::empty()git init with no commits

This was written by Claude Code on behalf of maximilian

Move the integration test `TestRepo` from `tests/common/mod.rs` into
`src/testing/mod.rs`, replacing the lightweight unit test version. One
canonical struct now serves both unit and integration tests.

Key changes:
- `_snapshot_guard: insta::...` → `_lifetime_guard: Option<Box<dyn Any>>`
  (type-erased, removes insta dependency from src/)
- Added `pub repo: Repository` field for unit test consumers
- `TestRepo::new()` = lightweight git init (unit tests)
- `TestRepo::standard()` = fixture copy with remote + worktrees (integration)
- `tests/common/` re-exports via `pub use worktrunk::testing::*`
- Moved `mock_commands` module to `src/testing/`
- Consolidated gitconfig creation via `write_test_gitconfig()`

Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
worktrunk-bot previously approved these changes Apr 7, 2026
@worktrunk-bot worktrunk-bot dismissed their stale review April 7, 2026 21:21

CI failed — TestRepo::new() missing local git identity

Copy link
Copy Markdown
Collaborator

@worktrunk-bot worktrunk-bot left a comment

Choose a reason for hiding this comment

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

CI fails: all commands::picker::summary::tests panic with "Author identity unknown" on t.repo.run_command(&["commit", ...]).

Root cause: The old src/testing.rs::TestRepo::new() set identity in the repo's local git config via repo.run_command(&["config", "user.name", "Test"]). The new TestRepo::new() writes identity to an external gitconfig file (write_test_gitconfig) and uses it via GIT_CONFIG_GLOBAL in configure_git_env. But Repository::run_command() doesn't set GIT_CONFIG_GLOBAL, so tests that commit via t.repo.run_command() fail.

Fix options:

  1. Have TestRepo::new() also set local git config identity (like the old version did), so repo.run_command() works without env setup
  2. Change the picker tests to use t.run_git() / t.git_command() instead of t.repo.run_command()

Option 1 is simplest and most backward-compatible — the tests are calling a reasonable API (t.repo.run_command) and shouldn't need to know about env isolation.

TestRepo::new() writes identity to an external gitconfig file accessed
via GIT_CONFIG_GLOBAL. But unit tests commit via repo.run_command()
which doesn't set that env var, causing "Author identity unknown" on CI.

Fix: also set user.name/user.email in the repo's local git config.

Co-Authored-By: Claude <noreply@anthropic.com>
The doctest can't resolve the function from a #[doc(hidden)] module.
The same logic is tested in the #[cfg(test)] mod below.

Co-Authored-By: Claude <noreply@anthropic.com>
@max-sixty max-sixty merged commit c03aa3a into main Apr 7, 2026
25 checks passed
@max-sixty max-sixty deleted the testing branch April 7, 2026 21:57
max-sixty added a commit that referenced this pull request Apr 8, 2026
…cover tests (#1997)

Deduplicate the ~20 lines of identical setup between `TestRepo::new()`
and `TestRepo::empty()` into a private `init_repo()` method. Migrate two
recover.rs tests that only need "a repo with a commit" from manual
`git_init() + set_test_identity()` to `TestRepo::with_initial_commit()`.

The remaining 8 recover tests keep their local pattern — they
intentionally create sibling/nested repo layouts within a shared tempdir
that TestRepo doesn't support.

Follow-up to #1991.

> _This was written by Claude Code on behalf of @max-sixty_

Co-authored-by: Claude <noreply@anthropic.com>
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