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

TELCOV10N-452 IMPL junit2obj filter plugin #510

Merged
merged 2 commits into from
Feb 25, 2025

Conversation

mvk
Copy link
Contributor

@mvk mvk commented Dec 18, 2024

  • make sure all the relevant TestSuite & TestCase attributes are properly passed into the json object
  • TODO: migrate to it in other contexts
SUMMARY

There is currently a similar filter named junit2dict
It does not take all the attributes from TestCase and TestSuite.
This filter converts JUnit XML reports to JSON format without losing attributes.
Hopefully it satisfies current junit2dict users so they can start using this one.

The safe migration process is proposed:

  1. use this PR in current reporting context, safely not breaking any code still using junit2dict
  2. stabilize json2obj, add tests and evaluate whether replacing junit2dict can be done
  3. once ready - move json2obj to json2dict
ISSUE TYPE
  • Enhanced Feature
Tests

Need help writing tests for a filter plugin.
I only verified the code is doing what it is supposed to, but did write any unit tests.
The code using this filter is not deployed yet, but it requires the filter.


TestDallasWorkload: preflight-green

@mvk mvk requested a review from a team as a code owner December 18, 2024 19:52
@dcibot
Copy link
Collaborator

dcibot commented Dec 18, 2024

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Dec 18, 2024

from change #510:

  • no check (not a code change)

Copy link

@ramperher
Copy link
Contributor

Hi @mvk , thanks for your contribution! I wanted to ask you, because in the PR, you are mentioning json2obj and json2dic, are they new filters to be included? Or are you referring to junit2obj and junit2dict?

Also, do you have any output of local tests where you tested this new filter?

Adding @tkrishtop in the look as she developed the junit2dict filter,.

@tkrishtop
Copy link
Contributor

Hello guys, json2dict filter is to be deprecated, we plan to move all Junit parsing into dci control server. cc: @fredericlepied @ylamgarchal

@mvk mvk force-pushed the mvk/filter_junit2obj branch from 95b6e42 to 5d69be5 Compare January 12, 2025 08:20
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

Copy link

@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

Copy link

@mvk
Copy link
Contributor Author

mvk commented Jan 12, 2025

@ramperher hi.
Just wanted to safely add converting of junit xml files to a json string, the older filter (junit2dict) was missiung some attributes, and I have added some tests.

@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

Copy link

@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

Copy link

@mvk mvk force-pushed the mvk/filter_junit2obj branch from 7254edf to 46462b6 Compare January 12, 2025 21:32
@mvk mvk requested review from a team as code owners January 12, 2025 21:32
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

Copy link

@mvk mvk force-pushed the mvk/filter_junit2obj branch from 46462b6 to 9090e32 Compare January 12, 2025 21:47
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

1 similar comment
@dcibot
Copy link
Collaborator

dcibot commented Jan 12, 2025

from change #510:

  • no check (not a code change)

Copy link

Copy link

@mvk mvk force-pushed the mvk/filter_junit2obj branch from 1677054 to a9c9877 Compare February 25, 2025 11:09
Copy link

@mvk mvk force-pushed the mvk/filter_junit2obj branch from a9c9877 to e01ee39 Compare February 25, 2025 11:16
Copy link

@dcibot
Copy link
Collaborator

dcibot commented Feb 25, 2025

@tkrishtop
Copy link
Contributor

retest

@dcibot
Copy link
Collaborator

dcibot commented Feb 25, 2025

Copy link

@tkrishtop
Copy link
Contributor

tkrishtop commented Feb 25, 2025

Hi @mvk sorry, I see that Python2 compatibility is required by our CI, let's bring those back:

## Regressions are listed below
-ERROR: plugins/filter/junit2obj.py: missing: from __future__ import (absolute_import, division, print_function)
-ERROR: plugins/filter/junit2obj.py: missing: __metaclass__ = type

Also please remove crucible team from the reviewers, add verified signature to the commit, and I believe we can merge.

@dcibot
Copy link
Collaborator

dcibot commented Feb 25, 2025

@mvk mvk force-pushed the mvk/filter_junit2obj branch from ce0f7ef to ff5e03f Compare February 25, 2025 15:11
Copy link

@mvk
Copy link
Contributor Author

mvk commented Feb 25, 2025

Hi @mvk sorry, I see that Python2 compatibility is required by our CI, let's bring those back:

## Regressions are listed below
-ERROR: plugins/filter/junit2obj.py: missing: from __future__ import (absolute_import, division, print_function)
-ERROR: plugins/filter/junit2obj.py: missing: __metaclass__ = type

Also please remove crucible team from the reviewers, add verified signature to the commit, and I believe we can merge.

done.

@tkrishtop tkrishtop removed the request for review from a team February 25, 2025 15:31
Copy link
Contributor

@tkrishtop tkrishtop left a comment

Choose a reason for hiding this comment

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

LGTM, please sign the commit and let's merge

mvk added 2 commits February 25, 2025 17:35
Motivation:
===========

It is more convenient to navigate data in splunk via json.
The existing filter `redhatci.ocp.json2dict` misses multiple
ibutes, has no tests, so this one collects more of them to properly
 into the resulting JSON object + is tested

TODO: migrate to it in other contexts

Details:
--------

1. Implement the filter `redhatci.ocp.junit2obj`
1. Add tools configuration for `pylint`, `pycodestyle`, et al. under
   `pyproject.toml`
1. Add `.gitignore` with `.docsible`
1. Implement unit test for the filter
    * fixtures under `tests/unit/data`
    * Add example data + output to the filter module docstring
    * test requirements under `tests/unit/requirements.txt`

Signed-off-by: Maxim Kovgan <makovgan@redhat.com>
Signed-off-by: Maxim Kovgan <makovgan@redhat.com>
@mvk mvk force-pushed the mvk/filter_junit2obj branch from ff5e03f to 61438c5 Compare February 25, 2025 15:36
Copy link

@tonyskapunk
Copy link
Contributor

retest

1 similar comment
@tonyskapunk
Copy link
Contributor

retest

@dcibot
Copy link
Collaborator

dcibot commented Feb 25, 2025

@mvk mvk added this pull request to the merge queue Feb 25, 2025
Merged via the queue into redhatci:main with commit 5f595f3 Feb 25, 2025
7 checks passed
@mvk mvk deleted the mvk/filter_junit2obj branch February 25, 2025 18:20
@mvk mvk mentioned this pull request Mar 5, 2025
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.

6 participants