Opened PRs for 4 new found and fixed tests #1208
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@zzjas please inspect the logs. |
|
Have you run these tests individually with NonDex? I could only find logs for Please run these tests individually and let me know the file names to check. |
|
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 GitHub<https://urldefense.com/v3/__https:/github.com/TestingResearchIllinois/idoft/pull/1208*issuecomment-1849107562__;Iw!!DZ3fjg!7_7S1W2MieKsgKrNYOe5_qs4Kt8gO6UR1_Tjo147qbETUp_yStdonlYM1pEmQ-fUgKDhoIr9Iq1r_aqXxRmZtoGVNwbMFQ$>, or unsubscribe<https://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: ***@***.***>
|
|
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 |
|
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. |
|
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: approvals/ApprovalTests.Java#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 GitHub<https://urldefense.com/v3/__https:/github.com/TestingResearchIllinois/idoft/pull/1208*issuecomment-1849185418__;Iw!!DZ3fjg!6ucTS6nvQ1Yv2dNzlyKZ1qf0zKM50Cidp_x0skttkoOMGSD9G3vl393hVrqgsBdIBvwlAcjaiJ7VYFeuVg321V2n_9kebA$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ALP2QD5BYCP5XJFPKYOWPM3YIZONXAVCNFSM6AAAAABAMAP6EKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBZGE4DKNBRHA__;!!DZ3fjg!6ucTS6nvQ1Yv2dNzlyKZ1qf0zKM50Cidp_x0skttkoOMGSD9G3vl393hVrqgsBdIBvwlAcjaiJ7VYFeuVg321V0NTjCZnA$>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VM: 097
Logs for Causeway: ~/final_project/causeway.
All logs are named with prefix "mvn-install, mvn-dtest, mvn-nondex".