TestingResearchIllinois / idoft

35 stars 248 forks source link

Opened PRs for 4 new found and fixed tests #1208

Closed wuh3 closed 11 months ago

wuh3 commented 11 months ago

VM: 097

Logs for Causeway: ~/final_project/causeway.

All logs are named with prefix "mvn-install, mvn-dtest, mvn-nondex".

darko-marinov commented 11 months ago

@zzjas please inspect the logs.

zzjas commented 11 months ago

Have you run these tests individually with NonDex? I could only find logs for org.apache.causeway.extensions.pdfjs.metamodel.PdfjsViewer_MixinDomain_IntegTest#dump_facets.

Please run these tests individually and let me know the file names to check.

wuh3 commented 11 months ago

Hello Zijie,

All of these four tests extended the same abstract class extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_Abstract_IntegTest.java. Running any of these 4 tests will call the same function dump_facets in the abstract class.

Best, Mike Wu

From: Zijie Zhao @.> Date: Sunday, December 10, 2023 at 4:30 PM To: TestingResearchIllinois/idoft @.> Cc: Wu, Mike @.>, Author @.> Subject: Re: [TestingResearchIllinois/idoft] Opened PRs for 4 new found and fixed tests (PR #1208)

Have you run these tests individually with NonDex? I could only find logs for org.apache.causeway.extensions.pdfjs.metamodel.PdfjsViewer_MixinDomain_IntegTest#dump_facets.

Please run these tests individually and let me know the file names to check.

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/TestingResearchIllinois/idoft/pull/1208*issuecomment-1849107562__;Iw!!DZ3fjg!7_7S1W2MieKsgKrNYOe5_qs4Kt8gO6UR1_Tjo147qbETUp_yStdonlYM1pEmQ-fUgKDhoIr9Iq1r_aqXxRmZtoGVNwbMFQ$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ALP2QD5B7UJLGX7EMAXHGE3YIYZXTAVCNFSM6AAAAABAMAP6EKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBZGEYDONJWGI__;!!DZ3fjg!7_7S1W2MieKsgKrNYOe5_qs4Kt8gO6UR1_Tjo147qbETUp_yStdonlYM1pEmQ-fUgKDhoIr9Iq1r_aqXxRmZtoHBkl131A$. You are receiving this because you authored the thread.Message ID: @.***>

zzjas commented 11 months ago

That's ok.

@darko-marinov could you take a look at the real PR, if that's ok then this PR is ok to merge

darko-marinov commented 11 months ago

Fine to accept this PR here. The real PR needs more follow up (what the developer asked to "normalize XML"); otherwise, we'll consider these as likely to be rejected and not accepted.

wuh3 commented 11 months ago

Dear Dr. Darko,

I have tried the developer suggestion. The problem is that the developer insisted on using the ApprovalTests. However, the ApprovalTests.verifyXml() only takes in a string. Normalizing the string will make it into an actual Document. If I convert it back to string to pass it into ApprovalTests, the order issue will persist. I did a lot of research, and I see many people have similar issues with the xml orders or JSON orders, mostly due to running same code on different OS. The closest one that I found was this one: https://github.com/approvals/ApprovalTests.Java/issues/301

The developer of ApprovalTests closed the issue and asserted that they will not fix the order issue because xml order inconsistency is common and unimportant, and regularly running ApprovalTests with inconsistent xml order will not cause error. Instead, they proposed the .withComparator() feature to allow programmers to implement custom Comparators in order to pass the order tests. I tried the .withComparator() method. I believe it is more suitable for JSON comparisons since it takes in a Comparator of File instead of String.

Therefore, I believe that’s why the Causeway developer replied that it is okay to not have a “fix” for these tests. But, I believed that I have committed enough efforts and learned a lot about both Causeway and ApprovalTests.

Best, Mike Wu

From: darko-marinov @.> Date: Sunday, December 10, 2023 at 7:26 PM To: TestingResearchIllinois/idoft @.> Cc: Wu, Mike @.>, Author @.> Subject: Re: [TestingResearchIllinois/idoft] Opened PRs for 4 new found and fixed tests (PR #1208)

Fine to accept this PR here. The real PR needs more follow up (what the developer asked to "normalize XML"); otherwise, we'll consider these as likely to be rejected and not accepted.

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/TestingResearchIllinois/idoft/pull/1208*issuecomment-1849185418__;Iw!!DZ3fjg!6ucTS6nvQ1Yv2dNzlyKZ1qf0zKM50Cidp_x0skttkoOMGSD9G3vl393hVrqgsBdIBvwlAcjaiJ7VYFeuVg321V2n_9kebA$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ALP2QD5BYCP5XJFPKYOWPM3YIZONXAVCNFSM6AAAAABAMAP6EKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBZGE4DKNBRHA__;!!DZ3fjg!6ucTS6nvQ1Yv2dNzlyKZ1qf0zKM50Cidp_x0skttkoOMGSD9G3vl393hVrqgsBdIBvwlAcjaiJ7VYFeuVg321V0NTjCZnA$. You are receiving this because you authored the thread.Message ID: @.***>