test: Test for duplicate imports #3267 and fqn bug

[moraispgsi]

Hey @monperrus so I finally created the test for the duplicated imports #3267 issue. I added the leafactorci code as usual and added a small test with an input file(A.java) and an expected file(B.java). The test is under \src\test\java\spoon\processing\ProcessingTest.java - testRedundantImports(). To see the problem just run the test and see the difference between the files.

[MartinWitt]

Hi,
if you mark your test with @Ignore("UnresolvedBug") the CI will check that the test fails and be green afterwards.

[moraispgsi]

Thanks for the reply @MartinWitt. I need this bug to be resolved. If I mark it as ignored won't the underlying problem be ignored as well? My intention is not to merge this test but to show the problem so that the Spoon team can debug it effectively.

[MartinWitt]

A merged testcase makes the workflow easier, because you don't have to checkout the pr to fix it. We created this annotation to merge failing testcases to master. The issue shouldn't be ignored, but a fix could take some time depending on the problem and the time from people.

[moraispgsi]

This test case is complex, it uses the codebase of my master thesis that @monperrus is familiar with. I don't think this test should be merged for two reasons. First is that the code is part of another project, second is that it has more than 30 files. I agree that approach but this is an edge case that I simply can't unitize. Thanks again @MartinWitt. All I need to publish my code is to have this issue resolved.

[MartinWitt]

Okay, then it makes sense to not merge it. May I ask you:

• Do you need to keep any deadline with this bug or is the thesis finished?
• Is @monperrus working on a fix/has looked into it ?

I hope i can find some time to look into it at the end of the week, if i got the other issue solved.

[monperrus]

@moraispgsi, thanks for the failing test case. @MartinWitt, I'm not working on it, it's great if you can have a look! Thanks you both!

[moraispgsi]

It would be great if it was fixed by the end of the month, the thesis is not written yet but the project implementation is near its end. After this bug fix all my unit tests will pass. There may be other smaller problems with Spoon(e.g. sometimes the indentation is wrong after a modification) but not as severe as this one and therefore can be posponed.

[moraispgsi]

Hey, any news on this issue @MartinWitt @monperrus?

[MartinWitt]

Currently i need to do some academic stuff and had no time to look into it sry.
Still trying to find time for this.

[monperrus]

I confirm I can reproduce the bug locally.

[monperrus]

Good news, I have a fix! See last commit.

Notes for having super useful failing test cases:

• your failing test makes more things than checking for duplicate imports: it'd be great if the failing test asserts one single thing
• your build fails more tests that are unrelated (MainTest.testTest): it'd be great if the build is not polluted by other failing tests

Thanks.

[moraispgsi]

Hey @monperrus, that is great to hear. There were two problem, the duplicated imports, and the fqn problem. Have you addressed the later as well? Also, I would make a simpler test but I just could not pinpoint the exact problem so I could not replicate it in a simpler test, that is why I had to bring that amount of code into the test, sorry for the trouble.
Thanks for your time.

[monperrus]

Let's be principled.

Do you confirm that #3267 is fixed? If yes, we'll extract the fix in another PR, merge it and close #3267.

Then, we proceed with the next bug.

[moraispgsi]

Sure. I can't test it right now, will do it as soon as I can.

[moraispgsi]

Hey @monperrus. I just tested it, the duplicated imports problem is completely fixed, good job. That just leaves the FQN problem, I suppose you want to create a new PR with this fix first and close the #3267 issue before proceeding with the next problem. Thanks again for the effort.

[monperrus]

#3388 extracts the fix from here.

for the FQN problem, the problem may be on your side, make sure to call setSimplyQualified(true) on the type references you create.

[moraispgsi]

I will give it a try and come back to you with the results. Thanks.