Skip to content

AVRO-3689: [lang/java/avro] Fix flaky test ‘testAnnotationMultiAvroMeta’ #2012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jinzhengxu wants to merge 2 commits into
base: main
Choose a base branch
from
Open

AVRO-3689: [lang/java/avro] Fix flaky test ‘testAnnotationMultiAvroMeta’ #2012

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c1cf5f
fix-flaky-test
Jinzhengxu Dec 15, 2022
96c206b
rewrite the test
Jinzhengxu Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,10 +632,15 @@ public static class RAvroMultiMeta {

@Test
public void testAnnotationMultiAvroMeta() {
check(RAvroMultiMeta.class,
"{\"type\":\"record\",\"name\":\"RAvroMultiMeta\",\"namespace\":"
+ "\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+ "{\"name\":\"a\",\"type\":\"int\",\"K\":\"V\",\"L\":\"W\"}]" + ",\"X\":\"Y\",\"A\":\"B\"}");
String schm = ReflectData.get().getSchema(RAvroMultiMeta.class).toString();
String expectedString = "{\"type\":\"record\",\"name\":\"RAvroMultiMeta\",\"namespace\":"
+ "\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+ "{\"name\":\"a\",\"type\":\"int\",\"K\":\"V\",\"L\":\"W\"}]" + ",\"X\":\"Y\",\"A\":\"B\"}";
char[] schmArrays = schm.toCharArray();
char[] expectedArrays = expectedString.toCharArray();
Arrays.sort(schmArrays);
Arrays.sort(expectedArrays);
assertEquals(new String(schmArrays), new String(expectedArrays));
Copy link
Contributor

@clesaec clesaec Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done, may be it should be nice to add nondex plugin in build.
For the fix, i would prefer standard schema comparison, like

    Field field = new Field("a", Schema.create(Schema.Type.INT));
    field.addProp("L", "W");
    field.addProp("K", "V");
    Schema avroMultiMeta = Schema.createRecord("RAvroMultiMeta", null, "org.apache.avro.reflect.TestReflect", false,
      Arrays.asList(field));
    avroMultiMeta.addProp("X", "Y");
    avroMultiMeta.addProp("A", "B");

    Schema schema = ReflectData.get().getSchema(RAvroMultiMeta.class);
    assertEquals(avroMultiMeta, schema);

(In same time, it test Schema.equals method).

Copy link
Contributor Author

@Jinzhengxu Jinzhengxu Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback on my pull request. In terms of the fix, I think using a standard schema comparison tool like the one you suggested would be a good idea. I will make the necessary changes and update the pull request accordingly. Thanks again for your suggestion!

Copy link
Contributor Author

@Jinzhengxu Jinzhengxu Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I agree that adding a nondex plugin to the build would be a good idea. Using nondex can improve the reliability and stability of the code, which can help developers ensure that code changes don't result in unexpected outcomes. I will submit a new pull request about adding nondex to Avro for review.

Here is the link to Nondex: https://github.com/TestingResearchIllinois/NonDex. The lastest version can support Java 9+ project.

}

public static class RAvroDuplicateFieldMeta {
Expand Down