Skip to content

Conversation

@max-sixty
Copy link
Collaborator

Summary

Refactors the --test-runner-fallback CLI flag to support a more ergonomic flag/no-flag pattern while maintaining full backward compatibility.

Changes

New ergonomic syntax:

  • --test-runner-fallback - Enable fallback (no value required)
  • --no-test-runner-fallback - Disable fallback

Backward compatible syntax (still works):

  • --test-runner-fallback=true
  • --test-runner-fallback=false

Implementation Details

  • Uses clap's num_args(0..=1) with default_missing_value = "true" to allow the flag to work both with and without an explicit value
  • require_equals = true ensures --test-runner-fallback=value syntax when a value is provided, preventing ambiguity with test names
  • Flags use overrides_with for proper mutual exclusion (last one wins)
  • Added test_runner_fallback_value() method to derive the effective boolean value from the two flag fields

Testing

  • ✅ All existing tests pass (220+ tests)
  • ✅ All lints pass (pre-commit)
  • ✅ Manual testing verified all syntaxes work correctly
  • ✅ Code reviewed by subagent with code-review-guidelines skill

Test Plan

  • Verify --test-runner-fallback enables fallback
  • Verify --test-runner-fallback=false disables fallback
  • Verify --test-runner-fallback=true enables fallback (backward compat)
  • Verify --no-test-runner-fallback disables fallback
  • Verify last flag wins when both are specified
  • Verify help text is clear and accurate

🤖 Generated with Claude Code

max-sixty and others added 3 commits October 28, 2025 11:56
Refactors the --test-runner-fallback CLI flag to support a more ergonomic
flag/no-flag pattern while maintaining backward compatibility.

New syntax:
- --test-runner-fallback (enable fallback, no value required)
- --no-test-runner-fallback (disable fallback)

Old syntax still works:
- --test-runner-fallback=true
- --test-runner-fallback=false

Implementation uses clap's num_args(0..=1) with default_missing_value to
allow the flag to work both with and without an explicit value. The
require_equals attribute ensures --test-runner-fallback=value syntax when
a value is provided, preventing ambiguity with test names.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Adds comprehensive tests to verify all flag syntaxes work correctly:
- --test-runner-fallback (enable fallback, no value)
- --test-runner-fallback=true (backward compat)
- --test-runner-fallback=false (disable with explicit value)
- --no-test-runner-fallback (ergonomic disable)
- Override behavior (last flag wins)

Tests ensure all flag combinations parse correctly and execute without
errors, validating the test plan requirements from the PR.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@max-sixty max-sixty merged commit dcc2e04 into mitsuhiko:master Oct 28, 2025
15 checks passed
@max-sixty max-sixty deleted the fallback-flag branch October 28, 2025 19:21
max-sixty pushed a commit to max-sixty/insta that referenced this pull request Nov 2, 2025
This is a small fix to PR mitsuhiko#811 which introduced `--test-runner-fallback`
and `--no-test-runner-fallback` flags.

## Issue

The previous PR added `require_equals = true` to the clap argument
configuration, which broke backward compatibility with the space syntax:

- ❌ `--test-runner-fallback true` (broke in mitsuhiko#811)
- ✅ `--test-runner-fallback=true` (still worked)

The old implementation accepted both syntaxes, so this was a regression.

## Fix

Removed `require_equals = true` from the argument configuration to restore
backward compatibility. This allows all syntaxes to work:

**New ergonomic syntax:**
- `--test-runner-fallback` (no value)
- `--no-test-runner-fallback`

**Backward compatible syntax:**
- `--test-runner-fallback true` (with space) ✅ now fixed
- `--test-runner-fallback=true` (with equals)
- `--test-runner-fallback false`
- `--test-runner-fallback=false`

## Testing

- Added new test `test_runner_fallback_space_true` to cover the space syntax
- All 7 test_runner_fallback tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
max-sixty added a commit that referenced this pull request Nov 2, 2025
This is a small fix to PR #811 which introduced `--test-runner-fallback`
and `--no-test-runner-fallback` flags.

## Issue

The previous PR added `require_equals = true` to the clap argument
configuration, which broke backward compatibility with the space syntax:

- ❌ `--test-runner-fallback true` (broke in #811)
- ✅ `--test-runner-fallback=true` (still worked)

The old implementation accepted both syntaxes, so this was a regression.

## Fix

Removed `require_equals = true` from the argument configuration to
restore backward compatibility. This allows all syntaxes to work:

**New ergonomic syntax:**
- `--test-runner-fallback` (no value)
- `--no-test-runner-fallback`

**Backward compatible syntax:**
- `--test-runner-fallback true` (with space) ✅ now fixed
- `--test-runner-fallback=true` (with equals)
- `--test-runner-fallback false`
- `--test-runner-fallback=false`

## Testing

- Added new test `test_runner_fallback_space_true` to cover the space
syntax
- All 7 test_runner_fallback tests pass

## Changes

- Removed `require_equals = true` from `cargo-insta/src/cli.rs`
- Added test for space syntax in
`cargo-insta/tests/functional/test_runner_fallback.rs`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Maximilian Roos <maximilian@Maximilians-MacBook-Pro.local>
Co-authored-by: Claude <noreply@anthropic.com>
@max-sixty max-sixty mentioned this pull request Nov 20, 2025
max-sixty added a commit that referenced this pull request Nov 20, 2025
## Summary

Prepare for the 1.44.0 release:

- Bump version to 1.44.0 in `insta/Cargo.toml` and
`cargo-insta/Cargo.toml`
- Update CHANGELOG.md with all changes since 1.43.2

## Changes in 1.44.0

- Added non-interactive snapshot review and reject modes for use in
non-TTY environments (LLMs, CI pipelines, scripts) #815
- Add `--disable-nextest-doctest` flag with deprecation warning #803
- Add ergonomic `--test-runner-fallback` / `--no-test-runner-fallback`
flags #811
- Apply redactions to snapshot metadata #813
- Remove confusing 'previously unseen snapshot' message #812
- Speed up JSON float rendering #806 (@nyurik)
- Allow globset version up to 0.4.16 #810 (@g0hl1n)
- Improve documentation #814 (@tshepang)
- Enforce starting newlines in assertions #563

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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.

1 participant