[RHELC-1606] Fix of multiple backup of one file

[hosekadam]

File can be present multiple times in the output of 'rpm -Va' command. The file is backed up only once.

Also, the unit test was updated to handle this situation.

Jira Issues:

• RHELC-1606

Checklist

• [x] PR has been tested manually in a VM (either author or reviewer)
• [ ] Jira issue has been made public if possible
• [x] [RHELC-] or [HMS-] is part of the PR title
• [ ] GitHub label has been added to help with Release notes
• [x] PR title explains the change from the user's point of view
• [ ] Code and tests are documented properly
• [ ] The commits are squashed to as few commits as possible (without losing data)
• [ ] When merged: Jira issue has been updated to Release Pending if relevant

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.01%. Comparing base (ab3bbdd) to head (0269506). Report is 4 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1267 +/- ## ========================================== - Coverage 95.83% 92.01% -3.83% ========================================== Files 55 55 Lines 4756 4757 +1 Branches 836 836 ========================================== - Hits 4558 4377 -181 - Misses 112 284 +172 - Partials 86 96 +10 ``` | [Flag](https://app.codecov.io/gh/oamg/convert2rhel/pull/1267/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oamg) | Coverage Δ | | |---|---|---| | [centos-linux-7](https://app.codecov.io/gh/oamg/convert2rhel/pull/1267/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oamg) | `?` | | | [centos-linux-8](https://app.codecov.io/gh/oamg/convert2rhel/pull/1267/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oamg) | `91.95% <100.00%> (+<0.01%)` | :arrow_up: | | [centos-linux-9](https://app.codecov.io/gh/oamg/convert2rhel/pull/1267/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oamg) | `92.00% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oamg#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[has-bot]

/packit test --labels tier0

---

Comment generated by an automation.

Log | Bot Usage

[bookwar]

/packit test --labels tier0

[Venefilyn]

@hosekadam I had forgotten that I even reviewed this, but I created a solution too without realizing we had a solution created already. What is stopping us from doing something like this?

https://github.com/Venefilyn/convert2rhel/commit/1e480493e74b7f2c672bd876348b26baf1c88bf8

[hosekadam]

@Venefilyn looking at it, I would say they are really similar. Maybe the mine solution is easier to read (but it's probably because I did that :D) - like just adding the backed up path to the list of already backed up paths (which we already use, just extending it).

And yours where we have in the paths to back up just unique paths. Maybe the memory usage will be lower in this case.

Just one case came to my mind - what if the status or type of file would differ, but the path would be the same? In that case it would be added to the list, right? But maybe this is a too much corner case...

But I would definitely use the unit tests from this PR, which checks the complete process of the backup and rollback, I think the unit test is good because it tests the real usage (and I spent some time on it :D). But I understand, with your solution it's probably not needed so much.

[Venefilyn]

what if the status or type of file would differ, but the path would be the same?

Good question. Feels like that would never occur though :thinking:

@bookwar @r0x0d WDYT

[hosekadam]

/packit test --labels tier0

[r0x0d]

what if the status or type of file would differ, but the path would be the same?

Good question. Feels like that would never occur though 🤔

@bookwar @r0x0d WDYT

I would merge the two PRs in this one. I prefer @Venefilyn solution as it is simpler and does not require always updating a list after each new backed up file. Ideally, we should receive the data in that function already prepared to parse and backup.

But I agree with @hosekadam to keep his unit_tests as it is testing the whole situation. I would try to look for a way to split the test or simplify if possible. The test itself is very difficult to read :thinking:.

My vote here is then to pick @Venefilyn solution and @hosekadam unit_tests.

[Venefilyn]

We decided to merge this and my PR so closing this one