JSON Formatter for Result file (C++ and Python versions). Adds py Text report and py GithubCI report #191

MatteoRagni · 2024-09-30T07:53:55Z

Description

This commit introduces a new optional (and disabled by default) report format module for JSON file. Really similar to the Text Report module, uses nlohmann-json as dependency which is obtained automatically by CMake via FetchContent.

Alternatively, a python version of the json report tool shipped with the runtime is implemented.

To enable compilation of the module a specific option (ASAM_QC_JSON_REPORT_FORMAT_ENABLE) should be enabled in CMake at configuration time.

Side note, adding .vscode as ignored directory for the project.

Main changes

Adding a new report module subdirectory for report_module_json, with its own CMakeLists and dependency handling.
Added the subdirectory in report_modules CMakeLists, conditional to a specific cmake configuration option
Configuration option added with documentation in root CMakeLists file. Default value keeps the project as is.
Adding JSON report tool on python, using the new python API. The report is shipped alongside the runtime, inside theruntime.report.json module.
Adds implementation for text report in python (partially covers Replace C++ based TextReport module with Python implementation #115)
Adds working GithubCI report module in python

How was the PR tested?

Unit-test with some sample data. All unit tests passed.
Print output values. The printed outputs look reasonable.
Executed container locally and it worked.

Notes
Adding .vscode as ignored directory (.gitignore)

This commit introduces a new optional (and disabled by default) report format module for JSON file. Really similar to the Text Report module, uses nlohmann-json as dependency which is obtained automatically by CMake via `FetchContent`. To enable compilation of the module a specific option (`ASAM_QC_JSON_REPORT_FORMAT_ENABLE`) should be enabled in CMake at configuration time. Side note, adding `.vscode` as ignored directory for the project. Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

…into antemotion-mr/json-format

This commit introduces a new optional (and disabled by default) report format module for JSON file. Really similar to the Text Report module, uses nlohmann-json as dependency which is obtained automatically by CMake via `FetchContent`. To enable compilation of the module a specific option (`ASAM_QC_JSON_REPORT_FORMAT_ENABLE`) should be enabled in CMake at configuration time. Side note, adding `.vscode` as ignored directory for the project. Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

…into antemotion-mr/json-format

The json report tool can be started with either: * `qc-report-json` * `python -m runtime.report.json` Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

hoangtungdinh

Many thanks for the amazing work, @MatteoRagni. I have few minor comments below. Beside them, the only significant remark is that we can move the python module outside of the runtime module, to keep them decoupled.

Also, could you add an example output json file to README?

runtime/runtime/report/json/application.py

runtime/pyproject.toml

src/report_modules/report_module_json/dependencies/nlohman-json/CMakeLists.txt

Users can disable completely FetchContent with an option. In this case, nlohmann-json must be found via find_package (externally installed or vcpkg) Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

The commit introduces two base report classes: one to create the a custom formatter, the latter to wrap the formatter rapidly in an application. The approach has been tested on the json formatter. An initial scaffolding for the default text formatter has been provided, but implementation has not been done (prints only "not yet implemented") The approach implements also: - command line provided formatter parameters - support for reading from stdin - support for dump into stdout and stderr Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Report tool for github ci. It must exit with value different than 0 if issue are found, so that the CI can trigger the error. Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

hoangtungdinh

Hi @MatteoRagni , I think it would be great if we can replace the C++ Text Report with this Python Text Report (not sure if we manage to do it in v1.0.0-rc.1, but if not, we can have v1.0.0-rc.2 for that).

I just compared the output of the Python Text Report with the current C++ Text Report and have few comments below.

runtime/runtime/report/text/text_formatter.py

andreaskern74 · 2024-10-08T07:17:12Z

😮 @MatteoRagni, thanks for this huge contribution!

Some general notes:

This contribution is so big... I think you could additionally mention your company in the copyright header, because your are not commissioned by ASAM. Or for new files, depending how much IVEX now is contributing/reviewing, even only your company. This is a classic Open Source contribution.
Just scrolled 2min over the source code in VS Code to check if you missed a feature. Don't find anything, but I didn't check how the output looks like. But I stumbled over many typos and code findings... strongly recommend to use Flake8 and Code Spell Checker extension.
Nitpicking: IVEX create the subfolder runtime for the Python based runtime component, because src holds the C++ source code. Was not super happy, but I had no better idea at that moment. Now the complete Python source (including) different report modules is in runtime. Sight from outside looks now a little bit chaotic. But this is not important...

Thanks again.

MatteoRagni · 2024-10-08T08:06:32Z

This contribution is so big... I think you could additionally mention your company in the copyright header, because your are not commissioned by ASAM. Or for new files, depending how much IVEX now is contributing/reviewing, even only your company. This is a classic Open Source contribution.

Thanks for the suggestion. I still think this is a common effort, there is no direct need to cite company name.

Just scrolled 2min over the source code in VS Code to check if you missed a feature. Don't find anything, but I didn't check how the output looks like. But I stumbled over many typos and code findings... strongly recommend to use Flake8 and Code Spell Checker extension.

The project is configured for black as formatter, and the code is formatted according to its style. I'll take a look for the code spell checker.

Nitpicking: IVEX create the subfolder runtime for the Python based runtime component, because src holds the C++ source code. Was not super happy, but I had no better idea at that moment. Now the complete Python source (including) different report modules is in runtime. Sight from outside looks now a little bit chaotic. But this is not important...

The PR must take into account #195 which is just merged. I'll try to have a look at both integration and @hoangtungdinh comments above in a couple of days.

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

…m-ev#193) Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Co-authored-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

…m-ev#192) Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Co-authored-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Signed-off-by: romanodanilo <danilo@ivex.ai> Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com>

Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

The commit implements the following - considers Domain Specific Info in result as raw xml.etree Element, for string creation - includes the qc-report-text and qc-report-json in framework test - saves the framework unit test error output in one additional file (currently not stored in online pipelines). The file is added to gitignore Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni · 2024-10-30T12:26:22Z

I set the DCO to pass, it was wrongly recognized by the pipeline, even if signs were there.

Signed-off-by: Matteo Ragni <MatteoRagni@users.noreply.github.com>

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni · 2024-10-30T13:36:59Z

Many thanks for the amazing work, @MatteoRagni. I have few minor comments below. Beside them, the only significant remark is that we can move the python module outside of the runtime module, to keep them decoupled.

Also, could you add an example output json file to README?

I added documentation for the report utilities in manual/python_qc_framework

The PR is ready for merge (but revision is required)

@andreaskern74 @hoangtungdinh

hoangtungdinh

LGTM. Thanks for the great work, @MatteoRagni !

andreaskern74 · 2024-10-31T09:03:29Z

@hoangtungdinh : You want to merge/include it in rc2? If yes, then I need to take the time to look into the final behavior.... because currently nobody else has used it. So, there was no chance that people can give feedback.

The code looks good 👍 I've some worries about changing the functionality just before the release. I 💣-ed my products too often in life with merging cool stuff before the official release. 🙈

hoangtungdinh · 2024-10-31T09:24:16Z

I think it would be great if this change could be in the final release, but it is not really a must. I'm perfectly fine if we decide to move it to the next release.

The main reason for releasing it is that after this PR we will switch to the Python Text Report, and we plan to remove the C++ Text Report, addressing #115. Subsequent feedback can be incorporated directly into the Python Text Report, which is the future 😝.

I've tested the code locally once and it seems okay, but admittedly not intensively. In any case, I will release rc1 without this change first. And then we can decide whether to include it in rc2 or not.

MatteoRagni · 2025-01-08T13:27:06Z

Do we think we can move on with this?

andreaskern74 · 2025-01-08T14:14:19Z

Do we think we can move on with this?

Oh yes! I will directly look into the code that we can remove the current difficult parts as soon as possible.

andreaskern74 · 2025-01-08T14:21:53Z

src/report_modules/report_module_json/src/report_format_json.cpp

Do we really want to add both JSON report modules (Python and C++) to main? Does the C++ version have more features or some other reasons? The C++ JSON report modules feels deprecated before it have reached main.

Or maybe you want to add it, that if after the final switch to the Python based report modules then we could easier restore it?

Actually it is there only for historic reasons... The PR started with the C++ Json module, then @hoangtungdinh asked for a python version to deprecate the C++ libs, and it remained. I think I can make an update and completely remove it.

andreaskern74 · 2025-01-08T14:24:20Z

src/report_modules/report_module_json/README.md

+A schema of the output, with description of the fields is available in 
+[`schema/output-schema.json`](schema/output-schema.json).
+
+## Licenses for Dependencies


This should go to https://github.com/asam-ev/qc-framework/tree/main/licenses, in case we decide to add this C++ based report module to main.

andreaskern74 · 2025-01-08T14:27:16Z

src/report_modules/report_module_json/schema/output-schema.json

+  "properties": {
+    "inputFile": {
+      "type": "string",
+      "descrition": "Input file defined for the current run of the checker"


Suggested change

"descrition": "Input file defined for the current run of the checker"

"descrition": "Input file defined for the current run of the framework"

andreaskern74 · 2025-01-08T14:36:57Z

qc_framework/qc_framework/report/text/text_formatter.py

+        def p(val, indent=0):
+            if indent > 0:
+                print(" " * indent, file=output, flush=True, end="")
+            print(val, file=output, flush=True)
+
+        def S(indent=0): p(line_sep1, indent)
+
+        def s(indent=0): p(line_sep2, indent)
+
+        def n(indent=0): p("", indent)


I'm confused about the function names p, S, s, and n and what they are doing. It's the abracadabra xqar -> txt :-)

I know it's internal.... but can we get at least one line as comment? Is this a common approach I've never seen?

Ah, it's print, big/small separator and indented new line.

That means I need to comment a little more the code...

hoangtungdinh · 2025-01-08T17:07:25Z

I was checking the changes between the last time and now, and there is only one minor thing: If you could include the # SPDX-License-Identifier: MPL-2.0 to new files, as in this commit, that would be great @MatteoRagni.

MatteoRagni added 6 commits September 25, 2024 13:49

Merge remote-tracking branch 'MatteoRagni/antemotion-mr/json-format' …

038d151

…into antemotion-mr/json-format

Merge remote-tracking branch 'MatteoRagni/antemotion-mr/json-format' …

a18be8d

…into antemotion-mr/json-format

Adds JSON report tool (Python version)

14046b5

The json report tool can be started with either: * `qc-report-json` * `python -m runtime.report.json` Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni added the isState:New A new issue that needs to be classified to a type. label Sep 30, 2024

MatteoRagni requested review from andreaskern74, hoangtungdinh and AsamDiegoSanchez September 30, 2024 07:53

MatteoRagni self-assigned this Sep 30, 2024

MatteoRagni changed the title ~~Antemotion mr/json format~~ JSON Formatter for Result file (C++ and Python versions) Sep 30, 2024

hoangtungdinh reviewed Sep 30, 2024

View reviewed changes

runtime/runtime/report/json/application.py Outdated Show resolved Hide resolved

runtime/pyproject.toml Outdated Show resolved Hide resolved

src/report_modules/report_module_json/dependencies/nlohman-json/CMakeLists.txt Outdated Show resolved Hide resolved

MatteoRagni added 4 commits October 1, 2024 17:12

Optional to avoid automatic download of deps

dc09b86

Users can disable completely FetchContent with an option. In this case, nlohmann-json must be found via find_package (externally installed or vcpkg) Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Fixing mistype in CmakeFile preventing build

c9774df

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Just for fun, adding GithubCI formatter

f2ac6d6

Report tool for github ci. It must exit with value different than 0 if issue are found, so that the CI can trigger the error. Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni changed the title ~~JSON Formatter for Result file (C++ and Python versions)~~ JSON Formatter for Result file (C++ and Python versions). Adds py Text report (not implemented) and py GithubCI report Oct 1, 2024

MatteoRagni requested a review from hoangtungdinh October 1, 2024 16:09

Removing build date, that will be removed in v1.0.0-rc1

9b17fb0

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni mentioned this pull request Oct 3, 2024

Root python module runtime should be qc_framework. qc_framework should contain all the necessary to run the framework. #194

Closed

Completing text reporter, fixing issue on parameter automatic parsing

73a4419

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni changed the title ~~JSON Formatter for Result file (C++ and Python versions). Adds py Text report (not implemented) and py GithubCI report~~ JSON Formatter for Result file (C++ and Python versions). Adds py Text report and py GithubCI report Oct 4, 2024

This was referenced Oct 4, 2024

Solves #194 - Root module runtime #195

Merged

Replace C++ based TextReport module with Python implementation #115

Open

hoangtungdinh reviewed Oct 7, 2024

View reviewed changes

MatteoRagni added 2 commits October 8, 2024 11:41

Merge branch 'main' into antemotion-mr/json-format

5d8c60a

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Aligning report module with the new framework module directory structure

ef43499

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

andreaskern74 added the isState:Accepted An issue that has been accepted by the group, which needs to be assigned to a responsible label Oct 24, 2024

MatteoRagni and others added 12 commits October 30, 2024 13:19

Framework definitions for python report modules

7eb784e

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Add quotation to all windows paths to handle spaces in manifests (asa…

c682cdb

…m-ev#193) Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Co-authored-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Update esmini doc and add quotation to windows path in manifests (asa…

3d1c574

…m-ev#192) Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Co-authored-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Update windows build instructions (asam-ev#197)

0519ec7

Signed-off-by: romanodanilo <danilo@ivex.ai> Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com>

Update documentation and version

c15bd32

Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Add PyPi README for qc_framework

6f3850b

Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Build demo pipeline every week on Sunday at 2am

3256f94

Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Update xerces url

e493da9

Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Enforce rule uid in Cpp baselib and example checker bundle

a612c32

Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com>

Merging staged modifications to run the python report in unit tests

657155b

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

Restoring fix on windows manifest for tests

35d840c

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni and others added 2 commits October 30, 2024 13:30

Merge branch 'main' into antemotion-mr/json-format

8df5556

Signed-off-by: Matteo Ragni <MatteoRagni@users.noreply.github.com>

Including documentation for report utilities

c9ea3f6

Signed-off-by: Matteo Ragni <matteo.ragni@antemotion.com>

MatteoRagni requested a review from hoangtungdinh October 30, 2024 14:20

hoangtungdinh approved these changes Oct 31, 2024

View reviewed changes

andreaskern74 mentioned this pull request Dec 18, 2024

added lines argument to add_xml_location asam-ev/qc-baselib-py#51

Closed

andreaskern74 mentioned this pull request Jan 8, 2025

Batch process input files #209

Closed

andreaskern74 reviewed Jan 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSON Formatter for Result file (C++ and Python versions). Adds py Text report and py GithubCI report #191

JSON Formatter for Result file (C++ and Python versions). Adds py Text report and py GithubCI report #191

MatteoRagni commented Sep 30, 2024 •

edited

Loading

hoangtungdinh left a comment •

edited

Loading

hoangtungdinh left a comment •

edited

Loading

andreaskern74 commented Oct 8, 2024 •

edited

Loading

MatteoRagni commented Oct 8, 2024

MatteoRagni commented Oct 30, 2024 •

edited

Loading

MatteoRagni commented Oct 30, 2024

hoangtungdinh left a comment

andreaskern74 commented Oct 31, 2024

hoangtungdinh commented Oct 31, 2024

MatteoRagni commented Jan 8, 2025

andreaskern74 commented Jan 8, 2025

andreaskern74 Jan 8, 2025

MatteoRagni Jan 8, 2025

andreaskern74 Jan 8, 2025

andreaskern74 Jan 8, 2025

andreaskern74 Jan 8, 2025

andreaskern74 Jan 8, 2025

MatteoRagni Jan 8, 2025

hoangtungdinh commented Jan 8, 2025

	"descrition": "Input file defined for the current run of the checker"
	"descrition": "Input file defined for the current run of the framework"

JSON Formatter for Result file (C++ and Python versions). Adds py Text report and py GithubCI report #191

Are you sure you want to change the base?

JSON Formatter for Result file (C++ and Python versions). Adds py Text report and py GithubCI report #191

Conversation

MatteoRagni commented Sep 30, 2024 • edited Loading

hoangtungdinh left a comment • edited Loading

Choose a reason for hiding this comment

hoangtungdinh left a comment • edited Loading

Choose a reason for hiding this comment

andreaskern74 commented Oct 8, 2024 • edited Loading

MatteoRagni commented Oct 8, 2024

MatteoRagni commented Oct 30, 2024 • edited Loading

MatteoRagni commented Oct 30, 2024

hoangtungdinh left a comment

Choose a reason for hiding this comment

andreaskern74 commented Oct 31, 2024

hoangtungdinh commented Oct 31, 2024

MatteoRagni commented Jan 8, 2025

andreaskern74 commented Jan 8, 2025

andreaskern74 Jan 8, 2025

Choose a reason for hiding this comment

MatteoRagni Jan 8, 2025

Choose a reason for hiding this comment

andreaskern74 Jan 8, 2025

Choose a reason for hiding this comment

andreaskern74 Jan 8, 2025

Choose a reason for hiding this comment

andreaskern74 Jan 8, 2025

Choose a reason for hiding this comment

andreaskern74 Jan 8, 2025

Choose a reason for hiding this comment

MatteoRagni Jan 8, 2025

Choose a reason for hiding this comment

hoangtungdinh commented Jan 8, 2025

MatteoRagni commented Sep 30, 2024 •

edited

Loading

hoangtungdinh left a comment •

edited

Loading

hoangtungdinh left a comment •

edited

Loading

andreaskern74 commented Oct 8, 2024 •

edited

Loading

MatteoRagni commented Oct 30, 2024 •

edited

Loading