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.

Sign up for GitHub

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

Jump to bottom

Implement dynamic plugins and options [fix #135] #186

Merged
merged 2 commits into from
Apr 6, 2020
Merged

Implement dynamic plugins and options [fix #135] #186

merged 2 commits into from
Apr 6, 2020

Conversation

psss
Copy link
Collaborator

@psss psss commented Apr 1, 2020

Plugin autodetection from steps folders and user location.
Discover options dynamically detected from the plugins.
New module with method option handling and common options.
New plugin methods get() and default() to access plugin data.

@psss
Implement wake up for Run, Step and Discover
5e2e32e 
Add wake up functionality including Discover example.
Implement load() and save() for Run, Step and Discover.
Move status() from Common to Step where it belongs.
New exception FileError for file-related problems.
Test class now supports initialization from dict.
Implement --force thanks to Common._workdir_cleanup().
Rename dictionary_to_yaml() to dict_to_yaml().
@psss psss requested review from thrix and pvalena April 1, 2020 22:20
@psss psss added enhancement code | plugins Changes to the plugin implementation labels Apr 1, 2020
@lgtm-com
Copy link

lgtm-com bot commented Apr 1, 2020

This pull request fixes 1 alert when merging 2524b03 into 10f672e - view on LGTM.com

fixed alerts:

  • 1 for Mismatch between signature and use of an overriding method

thrix
thrix reviewed Apr 2, 2020
View reviewed changes
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

+- 50% done ....

@pvalena
Copy link
Collaborator

pvalena commented Apr 3, 2020 •
edited
Loading

That's a lot of pyhon for me, but it looks good.
I've mostly focused on the plugin part.

@psss
Copy link
Collaborator Author

psss commented Apr 3, 2020

@pvalena, thanks for the review.
@thrix, any other comments?

@lgtm-com
Copy link

lgtm-com bot commented Apr 3, 2020

This pull request fixes 1 alert when merging 4222e5d into c0fd856 - view on LGTM.com

fixed alerts:

  • 1 for Mismatch between signature and use of an overriding method

@thrix thrix self-requested a review April 6, 2020 12:52
@thrix
Copy link
Collaborator

thrix commented Apr 6, 2020

@psss any idea why there is two debug messaging about using DiscoverShell plugin?

$ cat rpminspect.fmf
summary: TMT/FMF plan for running rpminspect.

description: |
    To run analysis pass a build NVR in the `BUILD` environmnet variable.
    To run comparison, pass two NVRs, first is the `before` build, second one
    is the `after build`.

provision:
    how: container
    image: fedora

prepare:
    how: shell
    script: |
        if [ -z "$BUILD" ]; then
            echo "No build specified"
            exit 1
        fi
        dnf -y install dnf-plugins-core
        dnf -y copr enable dcantrell/rpminspect
        dnf -y install rpminspect

execute:
    how: shell
    script:
        - rpminspect $ARGS $BUILD

$ tmt run -i /var/tmp/tmt/run-048/ -de BUILD=lsvpd-1.7.11-1.el8 plan --name /rpminspect
Using tree '/home/mvadkert/git/github.com/generic-tests'.
/var/tmp/tmt/run-048/
Read file '/var/tmp/tmt/run-048/run.yaml'.
Found 1 plan.
Write file '/var/tmp/tmt/run-048/run.yaml'.

/rpminspect
summary: TMT/FMF plan for running rpminspect.
wake
    discover
        Read file '/var/tmp/tmt/run-048/rpminspect/discover/step.yaml'.
        Successfully loaded step data.
        status: done
        Read file '/var/tmp/tmt/run-048/rpminspect/discover/tests.yaml'.
        Using 'DiscoverShell' plugin for the 'shell' method.
        Using 'DiscoverShell' plugin for the 'shell' method.
        Discover wake up complete (already done before).
    provision
    prepare
        Read file '/var/tmp/tmt/run-048/rpminspect/prepare/step.yaml'.
        Step data not found.
    execute
        Read file '/var/tmp/tmt/run-048/rpminspect/execute/step.yaml'.
        Step data not found.
    report
        Read file '/var/tmp/tmt/run-048/rpminspect/report/step.yaml'.
        Step data not found.
    finish
        Read file '/var/tmp/tmt/run-048/rpminspect/finish/step.yaml'.
        Step data not found.
go
    discover
        workdir: /var/tmp/tmt/run-048/rpminspect/discover
...

thrix
thrix approved these changes Apr 6, 2020
View reviewed changes
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

LGTM, just found weird debug output, psss is lookin

@psss psss self-assigned this Apr 6, 2020
@psss
Copy link
Collaborator Author

psss commented Apr 6, 2020

@thrix, thanks for catching that. It was caused by the special handling of test discovery from the execute step. Moved into a separate method and now run only once based on the step status. Fixed in ca4bd88 but will squash into a single commit.

@psss
Implement dynamic plugins and options [fix #135]
1cd2cea 
Plugin autodetection from steps folders and user location.
Discover options dynamically detected from the plugins.
New module with method option handling and common options.
New plugin methods get() and default() to access plugin data.
@psss psss force-pushed the plugins-and-options branch from ca4bd88 to 1cd2cea Compare April 6, 2020 15:11
@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2020

This pull request fixes 1 alert when merging 1cd2cea into c0fd856 - view on LGTM.com

fixed alerts:

  • 1 for Mismatch between signature and use of an overriding method

@psss psss merged commit 00b92a0 into master Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrix thrix thrix approved these changes

@pvalena pvalena Awaiting requested review from pvalena

Assignees

@psss psss

Labels
code | plugins Changes to the plugin implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@psss @pvalena @thrix