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

[report-converter] Enable multiple input for report-converter #3897

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

bruntib
Copy link
Contributor

@bruntib bruntib commented Apr 28, 2023

Sometimes dynamic analysis is executed for the code base several times with different parameters. The output of these analyses may go to different log files which are given to report-converter one-by-one. If there are reports for the same source file in different log output files then the resulting .plist files will be overwritten at a subsequent execution of report-converter.

In this patch we make it possible to give multiple log files or directories containing log files to the report-converter. The reports in these files and directories will be collected all and will be converted to .plist without overriding the ones coming from a different log file.

Copy link
Contributor

@vodorok vodorok left a comment

Choose a reason for hiding this comment

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

Please see my remarks

# Type of "values" can have any of the indicated types above. But in
# argument parser it is given by "nargs='+'", so it will be a list in
# this specific case.
assert isinstance(values, Sequence)
Copy link
Contributor

Choose a reason for hiding this comment

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

The example about defining custom actions in the python documentation show that checking the number of items can be done in the constructor

def __init__(self, option_strings, dest, nargs=None, **kwargs):
        if nargs is None:
            raise ValueError("nargs needed")
        super().__init__(option_strings, dest, **kwargs)

https://docs.python.org/3/library/argparse.html#action
https://docs.python.org/3/library/argparse.html#argparse.Action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I didn't notice that there is a standard place for checking this precondition :)

@bruntib bruntib force-pushed the report_converter_multiple_inputs branch 12 times, most recently from 70ef995 to f6f0b73 Compare May 3, 2023 14:30
@bruntib bruntib requested a review from vodorok May 3, 2023 14:41
@bruntib bruntib force-pushed the report_converter_multiple_inputs branch from f6f0b73 to dddcd88 Compare June 19, 2023 12:09
Copy link
Contributor

@vodorok vodorok left a comment

Choose a reason for hiding this comment

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

LGTM

@bruntib bruntib force-pushed the report_converter_multiple_inputs branch from dddcd88 to 1bde6cc Compare June 20, 2023 13:53
Sometimes dynamic analysis is executed for the code base several times
with different parameters. The output of these analyses may go to
different log files which are given to report-converter one-by-one.
If there are reports for the same source file in different log output
files then the resulting .plist files will be overwritten at a
subsequent execution of report-converter.

In this patch we make it possible to give multiple log files or
directories containing log files to the report-converter. The reports
in these files and directories will be collected all and will be
converted to .plist without overriding the ones coming from a different
log file.
@bruntib bruntib force-pushed the report_converter_multiple_inputs branch from 1bde6cc to 0c62f8a Compare June 20, 2023 14:18
@bruntib bruntib merged commit b84ea42 into Ericsson:master Jun 20, 2023
@bruntib bruntib deleted the report_converter_multiple_inputs branch June 21, 2023 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants