Skip to content
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

WIP: GumTree simple #534

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

WIP: GumTree simple #534

wants to merge 6 commits into from

Conversation

jrfaller
Copy link

feat: update to latest gumtree-spoon, update to spoon gumtree-spoon version and replaced matcher with gumtree-simple.

…ersion and replaced matcher with gumtree-simple.
@jrfaller
Copy link
Author

Normally it compiles, however, I cannot run the tests but I am not sure why since it refers to uncompiled file errors, so I suspect this is a build environment or machine configuration problem 😢

@monperrus
Copy link
Collaborator

Revived CI, merged with latest master.

Seems that tests are failing because "because matcher is null"

@jrfaller
Copy link
Author

Thanks! OK, I think I fixed my classpath error, turns out IntelliJ was not setting the right source folders. I made a direct instantiation of the SimpleMatcher and fixed a bug when checking for root nodes. I think it's starting to work I had much more green tests! However, failures are very hard to debug...

@jrfaller
Copy link
Author

OK upon closer look most bugs seem to come from annotation keys. However not sure if it comes from replacing ClassicGumtree by SimpleIdGumTree or from the update of Spoon, maybe @slarse you could take a look? Cheers.

@jrfaller
Copy link
Author

jrfaller commented Oct 15, 2024

OK I have done more experiments.

By reverting to GumTree v4 classic and xy matchers, the following test cases do not work:

  • mergeToTree_shouldReturnExpectedTree_whenRightVersionIsModified (append_annotation_value_pair)
  • mergeToTree_shouldReturnExpectedTree_whenLeftVersionIsModified (append_annotation_value_pair)
  • mergeToTree_shouldReturnExpectedTree_whenBothVersionsAreModified (add_similar_methods)
  • mergeToTree_shouldReturnExpectedTree_whenBothVersionsAreModified (simple_root_conflict)

Using GumTree v4 simple id matcher

  • mergeToTree_shouldReturnExpectedTree_whenRightVersionIsModified (append_annotation_value_pair)
  • mergeToTree_shouldReturnExpectedTree_whenLeftVersionIsModified (append_annotation_value_pair)
  • mergeToTree_shouldReturnExpectedTree_whenBothVersionsAreModified (add_similar_methods)
  • mergeToTree_shouldReturnExpectedTree_whenBothVersionsAreModified (multiple_method_ordering_conflicts)
  • merge_shouldExitNonZero_onConflict (add_similar_fields)

Therefore it's pretty similar apart from the new failure on multiple_method_ordering_conflicts. I will try to run the code with GumTree v3 classic and xy matchers.

@slarse
Copy link
Collaborator

slarse commented Oct 17, 2024

@jrfaller I'm pretty unavailable this week and next, but I may be able to have a look Sunday.

Something to keep in mind is that some test cases expect very specific behavior. That they fail may not mean that the new result is wrong by definition, it may just be ever so slightly different. Writing non-brittle test cases for the merge was really quite difficult and I did not succeed at every turn. So some test cases are quite brittle :)

@jrfaller
Copy link
Author

Hi @slarse and glad to have some news! No pressure at all, just take a look if and when you feel like it ;-) For the annotation value pairs there is definitely a glitch since the key (and not the value) is lost in the merging process. But very hard to say where it comes from. Since it happens whatever the matcher, I suspect that this might be a Spoon-GT problem. I completely agree that test cases for merging tools are really hard to make in a not-fragile way, and you made a very good job with your test suite! Cheers.

@monperrus
Copy link
Collaborator

For the annotation value pairs there is definitely a glitch since the key (and not the value) is lost in the merging process.

Related to SpoonLabs/gumtree-spoon-ast-diff#221 (should be in 1.87 used in this PR)

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.

3 participants