Skip to content

Add integration tests for the file provider#4510

Merged
psss merged 5 commits intomainfrom
vaibhav-intx-file-provider
Feb 6, 2026
Merged

Add integration tests for the file provider#4510
psss merged 5 commits intomainfrom
vaibhav-intx-file-provider

Conversation

@vaibhavdaren
Copy link
Contributor

@vaibhavdaren vaibhavdaren commented Jan 19, 2026 •
edited by psss
Loading

Pull Request Checklist

  • implement the feature
  • Fix for single RPM file

Related to #4420.

@vaibhavdaren vaibhavdaren changed the title Added Intgeration Tests for File provider Added Integration Tests for File provider Jan 19, 2026
@happz happz force-pushed the vaibhav-artifact-install-integration-tests branch from b8f1dbe to f6ca434 Compare January 20, 2026 13:29
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-intx-file-provider branch from 5e1ce42 to 2b3099a Compare January 20, 2026 14:34
@happz happz force-pushed the vaibhav-artifact-install-integration-tests branch from f6ca434 to 3a9909b Compare January 20, 2026 14:35
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-intx-file-provider branch from 2b3099a to a64b81f Compare January 20, 2026 14:37
@vaibhavdaren vaibhavdaren self-assigned this Jan 20, 2026
@vaibhavdaren vaibhavdaren added this to the 1.66 milestone Jan 20, 2026
@github-project-automation github-project-automation bot moved this to backlog in planning Jan 20, 2026
@vaibhavdaren vaibhavdaren moved this from backlog to review in planning Jan 20, 2026
@vaibhavdaren vaibhavdaren added plugin | artifact Related to the `prepare/artifact` plugin. code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. labels Jan 20, 2026
@vaibhavdaren
Copy link
Contributor Author

vaibhavdaren commented Jan 20, 2026

Note: To be merged only after : #4449

@vaibhavdaren vaibhavdaren added code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. and removed code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. labels Jan 20, 2026
vaibhavdaren
vaibhavdaren commented Jan 20, 2026
View reviewed changes
@vaibhavdaren vaibhavdaren marked this pull request as ready for review January 20, 2026 18:26
@vaibhavdaren vaibhavdaren mentioned this pull request Jan 20, 2026
Closed
7 tasks
@happz happz added the status | blocked The merging of PR is blocked on some other issue label Jan 20, 2026
Base automatically changed from vaibhav-artifact-install-integration-tests to main January 20, 2026 20:42
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-intx-file-provider branch from a64b81f to d41630e Compare January 20, 2026 23:33
@vaibhavdaren vaibhavdaren added step | prepare Stuff related to the prepare step test coverage Improvements or additions to test coverage of tmt itself and removed status | blocked The merging of PR is blocked on some other issue labels Jan 21, 2026
@happz happz removed this from the 1.66 milestone Jan 27, 2026
@happz happz added the ci | full test Pull request is ready for the full test execution label Jan 29, 2026
@psss psss added this to the 1.67 milestone Jan 29, 2026
tcornell-bus
tcornell-bus reviewed Jan 29, 2026
View reviewed changes
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-intx-file-provider branch from bb6f77a to c5bfd62 Compare January 30, 2026 14:08
@psss psss mentioned this pull request Feb 4, 2026
Open
@LecrisUT
Copy link
Contributor

LecrisUT commented Feb 4, 2026

Changed the test to use simpler packages for now in 1f868a9.

Can you keep the older one that failed. A failing test is very valuable for investigation and designing how to handle the NVR parsing. If you have a simpler case that you could produce a similar failure, that would also be good to have.

@psss
Copy link
Contributor

psss commented Feb 4, 2026 •
edited
Loading

Can you keep the older one that failed. A failing test is very valuable for investigation and designing how to handle the NVR parsing. If you have a simpler case that you could produce a similar failure, that would also be good to have.

What about rather isolating the minimal reproducer (e.g. in a separate pull request) not to block this one?

@LecrisUT
Copy link
Contributor

LecrisUT commented Feb 4, 2026

What about rather isolating the minimal reproducer (e.g. in a separate pull request) not to block this one?

Yes, for sure. I mean keeping the older one anywhere, issue, notes, anything. Just not to loose the test case that we stumbled upon.

psss
psss requested changes Feb 4, 2026
View reviewed changes
Copy link
Contributor

@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.

Looks good. Just two suggestions.

psss
psss reviewed Feb 4, 2026
View reviewed changes
@vaibhavdaren
Copy link
Contributor Author

vaibhavdaren commented Feb 4, 2026

Can you keep the older one that failed. A failing test is very valuable for investigation and designing how to handle the NVR parsing. If you have a simpler case that you could produce a similar failure, that would also be good to have.

What about rather isolating the minimal reproducer (e.g. in a separate pull request) not to block this one?

Created #4549 for a minimal reproducer. So we do not loose the old test.

graphite-app[bot]
graphite-app bot reviewed Feb 4, 2026
View reviewed changes
LecrisUT
LecrisUT approved these changes Feb 4, 2026
View reviewed changes
Copy link
Contributor

@LecrisUT LecrisUT left a comment

Choose a reason for hiding this comment

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

Looks ok to me, but would prefer if we can avoid some duplication.

@vaibhavdaren vaibhavdaren force-pushed the vaibhav-intx-file-provider branch from 09a45e8 to 81c0444 Compare February 4, 2026 21:01
psss
psss approved these changes Feb 5, 2026
View reviewed changes
Copy link
Contributor

@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 the changes, now looks good.

Small hint for the commit summaries:

address review comment
address comments from code review

Choosing a few words describing the actual change helps to
identify what was fixed where and speeds up the review.

@psss
Copy link
Contributor

psss commented Feb 5, 2026

Tests are actually green. Failures are known eln container issue:

Ready for merging.

@psss psss moved this from review to merge in planning Feb 5, 2026
@psss psss force-pushed the vaibhav-intx-file-provider branch from 81c0444 to f168f82 Compare February 5, 2026 11:44
@happz happz force-pushed the vaibhav-intx-file-provider branch from f168f82 to f3f0713 Compare February 5, 2026 20:44
@psss
Copy link
Contributor

psss commented Feb 6, 2026

/packit build

@psss psss force-pushed the vaibhav-intx-file-provider branch from f3f0713 to 98c0244 Compare February 6, 2026 08:51
@psss
Copy link
Contributor

psss commented Feb 6, 2026

/packit build

2 similar comments
@psss
Copy link
Contributor

psss commented Feb 6, 2026

/packit build

@psss
Copy link
Contributor

psss commented Feb 6, 2026

/packit build

@psss psss force-pushed the vaibhav-intx-file-provider branch from 98c0244 to cf86708 Compare February 6, 2026 14:28
@psss psss merged commit ecf6ff8 into main Feb 6, 2026
28 checks passed
@psss psss deleted the vaibhav-intx-file-provider branch February 6, 2026 22:10
@github-project-automation github-project-automation bot moved this from merge to done in planning Feb 6, 2026
@vaibhavdaren vaibhavdaren mentioned this pull request Feb 8, 2026
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LecrisUT LecrisUT LecrisUT approved these changes

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

@psss psss psss approved these changes

@tcornell-bus tcornell-bus tcornell-bus approved these changes

Assignees

@vaibhavdaren vaibhavdaren

Labels

ci | full test Pull request is ready for the full test execution code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. plugin | artifact Related to the `prepare/artifact` plugin. step | prepare Stuff related to the prepare step test coverage Improvements or additions to test coverage of tmt itself

Projects

planning
Status: done

Milestone

1.67

Development

Successfully merging this pull request may close these issues.

5 participants

@vaibhavdaren @psss @LecrisUT @tcornell-bus @happz