test: add command line test for multi-search-replace #2220
Conversation
Adds a new test case that allows testing the multi-search-replace diff strategy directly from the command line by passing source and diff files as arguments. This enables easier debugging and manual testing of the diff application functionality without needing to modify test code for each test case. Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
|
|
See also: #1234 (comment) |
| } | ||
| } | ||
|
|
8000
||
| if (!sourceFile || !diffFile) { |
There was a problem hiding this comment.
Avoid silently returning when required file arguments are missing. Instead, consider using test.skip or explicitly failing so that missing arguments don't yield a false positive.
| `=============================== END ================================\n`, | ||
| ) | ||
| expect(result.success).toBe(true) | ||
| } else { |
There was a problem hiding this comment.
Ensure the test fails when diff application is unsuccessful by adding an assertion or calling fail() in the else branch. Currently, the else branch only logs output without failing the test.
| process.stdout.write( | ||
| `====================================================================\n`, | ||
| ) | ||
| process.stdout.write(result.content + "\n") |
There was a problem hiding this comment.
Hey @KJ7LNW, for this line where result.content is printed: does it make sense to print the whole content here since it might be long? Or perhaps print only the first 50 lines, similar to how the input source (line 1411) and diff (line 1430) are truncated?
There was a problem hiding this comment.
This is really a debugging tool wrapped in jest, not a test itself: this is just for debugging for direct user interact , it is not intended for automated testing.
I wrote this because I was troubleshooting some apply_diff problems and I have been collecting you failed samples from a number of bug reports. In order to understand the issue when a diff fails to apply I almost always need to see the entire output.
Maybe the line limit should be trimmed, but I specifically remember not wanting it to trim: the model originally wrote this trimming the output, but I had to tell it not to because I needed to see it all at once to compare what was happening.
|
Looks good to me, this is useful for quickly testing changes to the multi-search-replace tool whenever someone decides to improve it. |
Context
This PR will address apply_diff related issues mentioned in #1713. We've added a command line test capability to the multi-search-replace diff strategy to start trying out known issues.
Implementation
So far just bootstrapping test harness. Added a new test case that allows testing the multi-search-replace diff strategy directly from the command line by passing source and diff files as arguments. This enables easier debugging and manual testing of the diff application functionality without needing to modify test code for each test case.
The test can be run with:
How to Test
Get in Touch
Discord: KJ7LNW
Will help with #1713
Important
Adds command line test for
MultiSearchReplaceDiffStrategyinmulti-search-replace.test.tsto process diffs using command line arguments.multi-search-replace.test.tsforMultiSearchReplaceDiffStrategy.--sourceand--diffcommand line arguments.npx jest src/core/diff/strategies/__tests__/multi-search-replace.test.ts -t 'should process diff' -- --source <file.ts> --diff <diff.diff>.This description was created by
for 37e83d5. You can customize this summary. It will automatically update as commits are pushed.