Skip to content

Conversation

@xinying-7
Copy link
Contributor

@xinying-7 xinying-7 commented Nov 16, 2024

Opened 24 PRs for fixing swagger-api/swaggeer-core. The tests are failing NonDex since it's directly comparing JSON Strings which can have different key order, so I converted the actual and expected JSON String into JSON objects and compare with JSONAssert.assertEquals, which ensures correct equality checks regardless of key order.

@xinying-7 xinying-7 changed the title Opened a PR for fixing swagger-api/swagger-core: 'testFormatOfDecimal' Opened 14 PRs for test cases in swagger-api/swaggeer-core. Nov 16, 2024
@darko-marinov
Copy link
Contributor

For the real PR, please describe more as you're running a risk to have swagger-api/swagger-core#4783 rejected. @Ajitesh27 can you please help here?

For this PR on IDoFT, please squash the 10 commits into 1, and I can then accept this PR right away.

@Ajitesh27
Copy link

@xinying-7 In your real PR swagger-api/swagger-core#4783, please add more details about where the test is failing, why the failure is happening, how to reproduce it, how you fixed it, how to verify that your changes fixed it, etc.
You can use these PRs as reference:
jdereg/json-io#295
jdereg/json-io#301

@xinying-7
Copy link
Contributor Author

I've squashed the 10 commits into 1, and also changed the real PR description. Thanks for the references!

@darko-marinov
Copy link
Contributor

You currently have 2 commits not 1.

 Opened a PR for fixing swagger-api/swagger-core: 'serializeDoubleProperty'

 Opened a PR for fixing swagger-api/swagger-core: 'testFormatOfBigDecimal'

 Opened a PR for fixing swagger-api/swagger-core: 'testExtensionObjectWithProperties'

 Opened a PR for fixing swagger-api/swagger-core: 'serializeReadOnlyStringProperty'

 Opened 2 PRs for fixing swagger-api/swagger-core: 'serializeArrayModel' and 'deserializeArrayModel'

 Opened 2 PRs for fixing swagger-api/swagger-core: 'serializeArrayStringProperty' and 'deserializeArrayStringProperty'

 Opened 2 PRs for fixing swagger-api/swagger-core: 'serializeDateTimeProperty' and 'deserializeDateTimeProperty'

 Opened 2 PRs for fixing swagger-api/swagger-core: 'serializeLongMapProperty' and 'deserializeLongMapProperty'

 Opened a PR for fixing swagger-api/swagger-core: 'serializeObjectPropertyWithRequiredProperties'

Opened PRs for 10 tests in swagger-api/swagger-core
@xinying-7
Copy link
Contributor Author

Oh I separated the second one since it is this week's progress. I've squashed them all now.

@xinying-7
Copy link
Contributor Author

Also do I need to squash commits into 1 in real PRs as well?

@darko-marinov
Copy link
Contributor

I don't know what policy the real project has, but it may be good to squash for them if no one reviewed your PR there.

@darko-marinov darko-marinov merged commit 2dbe5b1 into TestingResearchIllinois:main Nov 22, 2024
1 check passed
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