-
-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tooling for better copy detection #1529
Conversation
7251db7
to
f67249b
Compare
…IDEs. Given it's marginal effect on the binary size, the negative effects on RustRover at least, and the added complexity, I think that ultimately it's not worth keeping, unfortunately.
Its main purpose is to help build test-cases from real-world repositories.
f67249b
to
4cf6d96
Compare
@@ -378,6 +379,84 @@ mod track_rewrites { | |||
Ok(()) | |||
} | |||
|
|||
#[test] | |||
fn jj_realistic_needs_to_be_more_clever() -> crate::Result { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the complexity of setting up the diffing in the tracker, I decided to add the test on gix
level instead of on plumbing level, where everything is far more 'simulated'.
The benefit is that here one can define the final result exactly as one would want to see it, but with the downside that the implementation actually happens elsewhere, i.e. in the tracker, which has its own set of tests which should probably also be amended with boiled-down versions of this once the algorithm is adjusted.
One assume that that the algorithm changes will already change the outcome of the existing tests.
… shell script. The shell-script will reproduce the repository, as long as the history is linear.
4cf6d96
to
1c59e82
Compare
1c59e82
to
7a17894
Compare
The pathspec list to reduce the set of files was generated with ``` git diff 47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3~1 47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3 --stat --no-renames --name-only ``` The assets and script to reproduce the fixture was created with: ``` cargo run -p internal-tools -- git-to-sh -c2 /Users/byron/dev/github.com/martinvonz/jj assets/jj-trackcopy-1 47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3 CHANGELOG.md cli/src/commands/cat.rs cli/src/commands/chmod.rs cli/src/commands/file/chmod.rs cli/src/commands/file/mod.rs cli/src/commands/file/print.rs cli/src/commands/mod.rs cli/tests/[email protected] cli/tests/runner.rs cli/tests/test_acls.rs cli/tests/test_cat_command.rs cli/tests/test_chmod_command.rs cli/tests/test_diffedit_command.rs cli/tests/test_file_chmod_command.rs cli/tests/test_file_print_command.rs cli/tests/test_fix_command.rs cli/tests/test_global_opts.rs cli/tests/test_immutable_commits.rs cli/tests/test_move_command.rs cli/tests/test_new_command.rs cli/tests/test_squash_command.rs cli/tests/test_unsquash_command.rs ``` It's notable that such issues are currently expected as the tracker implementation isn't as precise as the one in Git, which tracks more than one candidate.
7a17894
to
7ef1e88
Compare
A PR to add tooling and a test created with it to faciliate changes to the copy-detection algorithm.
Related to #1524
Tasks
Documenting the current behaviour would allow changes to be expressed and then implemented.
Unfortuantely, for reasons of convenience, the test is on a higher level than the implementation has to be.