Codex/fix playback issues with parameterized tests#71
Conversation
sloppylopez
changed the base branch from
main
to
codex/fix-404-in-fullflowflexibleit-test
There was a problem hiding this comment.
Pull request overview
This PR updates StableMock’s test fixtures and annotation surface to address playback behavior in parameterized/integration tests, and introduces a new dontIgnore override to keep specific fields from being ignored.
Changes:
- Add
dontIgnore()to@Ufor overriding both auto-detected ignore patterns and explicitignore()patterns. - Add a JUnit test intended to validate dontIgnore semantics (currently implemented as a local list-manipulation test).
- Re-record/refresh many example WireMock mappings and analysis artifacts used by the Spring Boot example tests (new UUIDs, updated timestamps/headers, added/removed body files).
Reviewed changes
Copilot reviewed 189 out of 243 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/main/java/com/stablemock/U.java |
Adds dontIgnore() annotation attribute and Javadoc. |
src/test/java/com/stablemock/core/analysis/AnalysisResultStorageDontIgnoreTest.java |
Adds a test intended to cover dontIgnore behavior (but currently doesn’t exercise production code). |
examples/spring-boot-example/src/test/resources/** |
Updates recorded mappings / bodies / detected-fields / analysis request logs for playback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class AnalysisResultStorageDontIgnoreTest { | ||
|
|
||
| @Test | ||
| void dontIgnoreEntriesAreRemovedFromEffectiveIgnoreSet() { | ||
| List<String> autoDetected = new ArrayList<>(Arrays.asList( |
There was a problem hiding this comment.
This test currently re-implements the ignore/dontIgnore merge logic with local lists instead of exercising the production code path that computes and applies effective ignore patterns. As written, it can pass even if the real playback logic is broken or if pattern normalization differs. Consider asserting behavior via the real integration point (e.g., the component that applies annotation ignore/dontIgnore to auto-detected patterns) and use the same pattern prefix conventions the runtime expects.
| /** | ||
| * Explicit protect list – these paths must not be ignored. | ||
| * Entries here override both auto-detected ignore patterns and {@link #ignore()}. | ||
| * Typical usage is to list identifying fields that should remain part of matching | ||
| * even if they look dynamic to the detector. | ||
| */ | ||
| String[] dontIgnore() default {}; |
There was a problem hiding this comment.
The new dontIgnore() option documents the intent, but it doesn’t document the expected pattern formats (e.g., json:, gql:/graphql:, xml://) the same way ignore() does. Without that, users can easily provide values that will never match and assume the override is broken. Please extend this Javadoc to explicitly state that dontIgnore entries must use the same prefixes/format as ignore patterns and auto-detected ignore patterns.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@sloppylopez I've opened a new pull request, #72, to work on those changes. Once the pull request is ready, I'll request review from you. |
sloppylopez
merged commit fb71abf
into
codex/fix-404-in-fullflowflexibleit-test
Description
Brief description of the changes in this PR.
Type of Change
Related Issue
Fixes #(issue number)
Changes Made
Testing
Checklist
Screenshots (if applicable)
Add screenshots to help explain your changes.
Additional Notes
Any additional information that reviewers should know.