wtrzas2 opened a new pull request, #4635:
URL: https://github.com/apache/servicecomb-java-chassis/pull/4635

   ### Purpose of this PR
   This PR fixes the flaky tests:
   
   
`org.apache.servicecomb.codec.protobuf.schema.TestSchemaToProtoGenerator.testListMapTypeCorrect`
   
`org.apache.servicecomb.codec.protobuf.schema.TestSchemaToProtoGenerator.test_springmvc_model_schema_correct`
   
   The mentioned tests may non-deterministically pass or fail without changes 
made to the source code when it is run in different JVMs due to the inherent 
unordered nature of the fields within the message part of the schema being 
tested.
   
   The first test expects part of the schema to be:
   `
   message DeptInfo {
     string name = 1;
     string code = 2;
     repeated ScoreInfo scores = 3;
   }
   `
   
   But it will sometimes return part of the schema as:
   ```
   message DeptInfo {
     string name = 1;
     repeated ScoreInfo scores = 2;
     string code = 3;
   }
   ```
   or 
   ```
   message DeptInfo {
     string code = 1;
     string name = 2;
     repeated ScoreInfo scores = 3;
   }
   ```
   
   The second test expects part of the schema to be:
   ```
   message Model {
             string name = 1;
             sint32 age = 2;
    }
   ```
   
   But it will sometimes return part of the schema as:
   ```
   message Model {
     sint32 age = 1;
     string name = 2;
   }
   ```
   
   ### Steps to Reproduce
   This change was confirmed by running the 
[NonDex](https://github.com/TestingResearchIllinois/NonDex) tool, which 
explores and reports errors in different behaviors of under-determined Java 
APIs.
   
   # Compile the module in project directory
   ```
   mvn clean install -pl common/common-protobuf -am -DskipTests
   ```
   
   # Run the unit tests using NonDex
   ```
   mvn edu.illinois:nondex-maven-plugin:2.1.7:nondex -pl common/common-protobuf 
-Dtest=org.apache.servicecomb.codec.protobuf.schema.TestSchemaToProtoGenerator#testListMapTypeCorrect
   ```
   
   ```
   mvn edu.illinois:nondex-maven-plugin:2.1.7:nondex -pl common/common-protobuf 
-Dtest=org.apache.servicecomb.codec.protobuf.schema.TestSchemaToProtoGenerator#test_springmvc_model_schema_correct
   ```
   
   Following the above steps for the mentioned tests will produce the following 
results for those runs, respectively:
   ```
   [ERROR]   TestSchemaToProtoGenerator.testListMapTypeCorrect:261 expected: 
<syntax = "proto3";
   package test.model;
   
   message ScoreInfo {
     sint32 type = 1;
   }
   
   message DeptInfo {
     string name = 1;
     string code = 2;
     repeated ScoreInfo scores = 3;
   }
   
   //@WrapProperty
   message ListScoreInfo {
     repeated ScoreInfo value = 1;
   }
   
   //@WrapProperty
   message ListDeptInfo {
     repeated DeptInfo value = 1;
   }
   
   //@WrapProperty
   message request {
     repeated DeptInfo value = 1;
   }> but was: <syntax = "proto3";
   package test.model;
   
   message ScoreInfo {
     sint32 type = 1;
   }
   
   message DeptInfo {
     string code = 1;
     string name = 2;
     repeated ScoreInfo scores = 3;
   }
   
   //@WrapProperty
   message ListScoreInfo {
     repeated ScoreInfo value = 1;
   }
   
   //@WrapProperty
   message ListDeptInfo {
     repeated DeptInfo value = 1;
   }
   
   //@WrapProperty
   message request {
     repeated DeptInfo value = 1;
   }>
   [INFO] 
   [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
   ```
   
   ```
   [ERROR]   TestSchemaToProtoGenerator.test_springmvc_model_schema_correct:152 
expected: <syntax = "proto3";
   package test.model;
   
   message Model {
     string name = 1;
     sint32 age = 2;
   }
   
   //@WrapProperty
   message testModelRequest {
     Model value = 1;
   }> but was: <syntax = "proto3";
   package test.model;
   
   message Model {
     sint32 age = 1;
     string name = 2;
   }
   
   //@WrapProperty
   message testModelRequest {
     Model value = 1;
   }>
   [INFO] 
   [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
   ```
   
   
   ### Fix
   I propose converting the set of properties (originally a `Map`), into a 
`List` and sorting the entries by key alphabetically by schema name (within 
`createMessage(Schema<?> schema)` method in `SchemaToProntoGenerator` class), 
utilizing Comparator to do so and then iterating over the sorted entries. To 
accommodate the alphabetical order, the expected ProtoSchema in the assertion 
within both tests reflects the alphabetical ordering as well.
   
   After the fix, I was able to get both test cases to pass on NonDex:
   <img width="949" alt="Screenshot 2024-12-05 at 10 04 18 PM" 
src="https://github.com/user-attachments/assets/95b12366-86a1-4225-b31d-1c03fc76f376";>
   
   <img width="945" alt="Screenshot 2024-12-05 at 10 06 17 PM" 
src="https://github.com/user-attachments/assets/3685fe0f-c2c8-40ab-893d-7d0cef09c725";>
   
   
   Please let me know if there are any changes you would like me to make.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to