Skip to content

MDBF-991 - Obtain failed test results and the associated output from MTR - #816

Merged
RazvanLiviuVarzaru merged 5 commits into
MariaDB:devfrom
RazvanLiviuVarzaru:MDBF-991
Aug 18, 2025
Merged

MDBF-991 - Obtain failed test results and the associated output from MTR#816
RazvanLiviuVarzaru merged 5 commits into
MariaDB:devfrom
RazvanLiviuVarzaru:MDBF-991

Conversation

@RazvanLiviuVarzaru

Copy link
Copy Markdown
Collaborator

Client side implementation of the MTR reporter.
Follow along commit messages for implementation details.

@RazvanLiviuVarzaru
RazvanLiviuVarzaru force-pushed the MDBF-991 branch 4 times, most recently from a638aa9 to e3fabbd Compare July 31, 2025 07:16
@fauust

fauust commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator

Looks good!
Some bash comments.

The syntax in bash for comparing integer should be if ((var != 0)) instead of if [[ $var -ne 0 ]]. It seems to me that this is much more close from what other languages do and someone not familiar with shell or bash would understand it easier. Also this is what https://google.github.io/styleguide/shellguide.html#testing-strings recommends which is a very good guide.

I am wondering if we should not use getopts for arguments, instead of positional arguments, the later seems more confusing and error prone to me (but maybe more complex to call from BB python?).

Finally, you should probably have an error function since error should go to STDERR, see https://github.com/MariaDB/buildbot/blob/dev/scripts/bash_lib.sh#L37-L41.

Those are not mandatory changes of course but nice to have I think.

Comment thread configuration/steps/commands/scripts/mtr_reporter.sh
@RazvanLiviuVarzaru
RazvanLiviuVarzaru force-pushed the MDBF-991 branch 3 times, most recently from d2d9c90 to 68ceebd Compare July 31, 2025 13:25
@RazvanLiviuVarzaru

RazvanLiviuVarzaru commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator Author

Thanks @fauust for the review! See: 68ceebd
About getops, might be a bit more complex to call it like that from bb python code, nothing impossible.
I don't expect the positional arguments of these scripts to change often.

PS: I cannot source the functions because the code is injected into a python string. The advantage is that the scripts are very portable, the disadvantage is obvious.
Is it not something I want to fix now but to keep in mind.

Sometimes is useful, for non-critical buildbot steps,
to mark them as warnings if they fail.

We don't know in advance what is the non-zero return code
of a script that marks a failure so is better to treat the 1-255 range
as a Warning and only 0 - Success.
do not use interpolate for load_script. The only way variables can be passed to
scripts is through command line arguments.
Standalone build step that checks for xml's files in the mtr log path
and reports them back to the collector service.

The collector service is running on the buildmaster as a
docker container and its address is defined in the .env files

This is not a critical client service so just warn on step failure.
Common failures that can occur are: collector service is not available or the report is not a valid xml.

If all MTR steps are successful then naturally there's no failure to report
from the XML files so skip this step -> hasFailed(step)
@RazvanLiviuVarzaru
RazvanLiviuVarzaru merged commit ca93c3f into MariaDB:dev Aug 18, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants