Skip to content
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.

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

Support serialization tests when generating ci.json #100

Merged
merged 2 commits into from
Jul 31, 2023

Conversation

generalmimon
Copy link
Member

@generalmimon
Copy link
Member Author

generalmimon commented Jul 30, 2023

Works only in Python so far. I took the path of least resistance, which turned out to be having separate targets python and python-write (at least from the perspective of the kst-adoption-report and aggregate/convert_to_json scripts). This means that Python test results will be in two folders: the traditional parse tests would remain in test_out/python (and to my knowledge, the ci.json stays exactly the same) and the serialization tests would go to test_out/python-write.

The test names in test_out/python-write/ci.json are special in that they include both the class name and the method name (eventually with stripped test prefixes / snake_case -> camelCase conversions), as discussed in kaitai-io/kaitai_struct#1060 (comment). For example:

{
  "$meta": {
    "lang": "python-write",
    "timestamp": "2023-07-31T10:02:40Z"
  },
  "BcdUserTypeBe.ReadWriteRoundtrip": {
    "status": "passed",
    "elapsed": 0.008,
    "is_kst": true
  },
  "BcdUserTypeLe.ReadWriteRoundtrip": {
    "status": "passed",
    "elapsed": 0.0,
    "is_kst": true
  },
  "BitsByteAligned.ReadWriteRoundtrip": {
    "status": "passed",
    "elapsed": 0.0,
    "is_kst": true
  },

KST adoption reporting is implemented too - if the test class is included in the kst_adoption.log file, all tests in that class will have "is_kst": true. If a class was in kst_adoption.log but there was no test method in it, a plain object { "is_kst": true } itself is added to ci.json under the pure class name as a key (this may be a bit unnecessary because I think such "test result" wouldn't show in our CI dashboard anyway, but the existing mode worked in a similar way).

@generalmimon generalmimon requested a review from GreyCat July 30, 2023 23:50
@generalmimon generalmimon force-pushed the serialization-tests-to-ci-json branch 2 times, most recently from 59d4282 to 4e0c5ed Compare July 31, 2023 09:20
Copy link
Member

@GreyCat GreyCat left a comment

Choose a reason for hiding this comment

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

Seems to work, so looks good to me!

Potentially I can outline 2 larger avenues of exploration:

  1. How we want to store/present tests for reading and writing parts altogether — e.g. for now "spec" + "specwrite" and "python" + "python-write" seems to be ok, but generally moving forward I really think we want to have them as first class citizens on the same level.

  2. Potentially looking into reworking Python invocation per se to report everything in one XML file (as many other languages do), i.e. https://github.com/xmlrunner/unittest-xml-reporting#reporting-to-a-single-file

@generalmimon generalmimon force-pushed the serialization-tests-to-ci-json branch from 4e0c5ed to aa79aa8 Compare July 31, 2023 09:48
@generalmimon
Copy link
Member Author

@GreyCat:

2. Potentially looking into reworking Python invocation per se to report everything in one XML file (as many other languages do), i.e. https://github.com/xmlrunner/unittest-xml-reporting#reporting-to-a-single-file

I agree that a single XML report would be better, but separate XML files work for now. And note that Java (and maybe other languages, haven't checked) uses separate XML files too.

@generalmimon
Copy link
Member Author

generalmimon commented Jul 31, 2023

@GreyCat I've just added Java support in a similar way.

@generalmimon generalmimon requested a review from GreyCat July 31, 2023 13:14
@generalmimon generalmimon merged commit 14309eb into serialization Jul 31, 2023
@generalmimon generalmimon deleted the serialization-tests-to-ci-json branch July 31, 2023 18:25
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.

2 participants