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

Add dynamic ref evaluation support to plan import #1833

Merged
merged 1 commit into from
Jun 30, 2023
Merged

Conversation

kkaarreell
Copy link
Collaborator

No description provided.

@kkaarreell
Copy link
Collaborator Author

This should address #1832
I will also add a test for this feature later.

@kkaarreell kkaarreell force-pushed the ksrot_plan_dynref branch 4 times, most recently from 6db26cc to a45f2f7 Compare February 5, 2023 12:29
tmt/base.py Outdated Show resolved Hide resolved
@kkaarreell
Copy link
Collaborator Author

kkaarreell commented Feb 6, 2023

One more thing I didn't like is the lack of information about the imported plan. I have added a commit improving this a bit, although I am really not sure the approach is correct (definitely not pretty).
The output looks like this.

    /plans/dynamic-ref
        discover
            how: fmf
            order: 50
            imported-url: https://github.com/teemtee/tmt
            imported-ref: fedora
            imported-path: /tests/discover/data
            imported-name: /plans/fmf/exclude
            directory: /var/tmp/tmt/run-344/import/plans/dynamic-ref/tests/discover/data
            summary: 2 tests selected
                /tests/discover2
                /tests/discover3

@kkaarreell kkaarreell force-pushed the ksrot_plan_dynref branch 7 times, most recently from 09a7449 to 5a091f3 Compare February 6, 2023 07:57
tmt/base.py Outdated Show resolved Hide resolved
tmt/base.py Outdated Show resolved Hide resolved
tmt/steps/discover/fmf.py Outdated Show resolved Hide resolved
tmt/steps/discover/shell.py Outdated Show resolved Hide resolved
@kkaarreell kkaarreell force-pushed the ksrot_plan_dynref branch 7 times, most recently from f4df534 to b1b0b9f Compare February 6, 2023 11:34
@thrix thrix self-requested a review April 13, 2023 14:26
@psss psss modified the milestones: 1.22, 1.23 Apr 14, 2023
@psss psss modified the milestones: 1.23, 1.24 Apr 25, 2023
@lukaszachy lukaszachy modified the milestones: 1.24, 1.25 May 16, 2023
@kkaarreell kkaarreell force-pushed the ksrot_plan_dynref branch 3 times, most recently from 39f2e39 to a2cde4c Compare June 8, 2023 08:42
@kkaarreell
Copy link
Collaborator Author

Hi @psss @happz @thrix ,
I have forced pushed a new version (since there were a lot of changes in main in the meantime), trying to follow the proposed approach.
What is not yet addressed:

  • the code branch # Use fmf cache for exploring plans (the whole git repo is not needed) where I am not sure what would be the right approach.. I guess I will wait for tmt tests to fail to see what is the impact
  • adding tests for this new functionality

@kkaarreell
Copy link
Collaborator Author

I have added back the test update so now there will be failures due to the newly added warning.
@psss you were suggesting to print the warning only for tmt plan show, how do I find out how tmt has been issued?

@kkaarreell
Copy link
Collaborator Author

OK, I did one more updated, adding extra git clone when we don't have enough data to evaluate dynamic ref. Still, it may be handy to avoid doing this when we do not need those data. WDYT about it?

@kkaarreell kkaarreell force-pushed the ksrot_plan_dynref branch 4 times, most recently from af3c9b6 to 1cd6444 Compare June 9, 2023 04:44
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Thanks for addressing comments, looks good. I've rebased on the latest main and added just a couple of minor changes in f9dcd90.

@psss psss self-assigned this Jun 30, 2023
@psss psss added discover Discover step base labels Jun 30, 2023
Add tests for test plan import with dynamic ref.
Clone repo if not enough data to eval dynamic ref.
Log import plan details.
@psss psss linked an issue Jun 30, 2023 that may be closed by this pull request
@psss psss merged commit 6baf6cb into main Jun 30, 2023
16 checks passed
@psss psss deleted the ksrot_plan_dynref branch June 30, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base discover Discover step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic ref evaluation doesn't work with remote plan
6 participants