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

Tests for rollback of group upgrade transaction rollback #1155

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

jan-kolarik
Copy link
Member

@jan-kolarik jan-kolarik force-pushed the jkolarik/group-upgrade-rollback-2-test branch from a4ccca3 to 029b418 Compare September 8, 2022 09:46
@jan-kolarik jan-kolarik marked this pull request as ready for review September 8, 2022 11:19
@pkratoch
Copy link
Contributor

The change allows not only the undo of the history upgrade rollback, but also the redo (so, the sequence of actions is: dnf group upgrade, dnf history undo last, dnf history redo last). It's probably not a common use case, but can you add tests also for that?

@jan-kolarik
Copy link
Member Author

jan-kolarik commented Sep 13, 2022

@pkratoch Thanks for the feedback. I was thinking about the sequence of actions you proposed - upgrade, undo last, redo last - and I don't understand what the last operation should mean? Intuitively when I do undo A I go to the time point before the A and when I want to undo that I will do the A operation again. That's fine and this is working now. But when you want to redo undo A it means "do nothing", right? 🙂 I've also check the similar sequence: install package, undo last, redo last for a single package with the current DNF version and it doesn't work either.

@pkratoch
Copy link
Contributor

@pkratoch Thanks for the feedback. I was thinking about the sequence of actions you proposed - upgrade, undo last, redo last - and I don't understand what the last operation should mean? Intuitively when I do undo A I go to the time point before the A and when I want to undo that I will do the A operation again. That's fine and this is working now. But when you want to redo undo A it means "do nothing", right? slightly_smiling_face

Yes, it's not usable in this context, but I imagine you could have scenario where you would want to replicate list of transactions on another system or you'd want to complete saved transaction or something like that (I remember an RFE for this, but I don't know if and how it was implemented at the end). In either case, you don't want to fail with error that the action is unknown (as it was before your patch).

I've also check the similar sequence: install package, undo last, redo last for a single package with the current DNF version and it doesn't work either.

Hm, I think it works, there is just nothing to do... or did I misunderstand what you meant?

# dnf install nano
# dnf history undo last
# dnf history redo last
Last metadata expiration check: 0:00:18 ago on Tue Sep 13 13:16:19 2022.
Dependencies resolved.
Nothing to do.
Complete!
Warning, the following problems occurred while running a transaction:
  Package nevra "nano-6.0-2.fc36.x86_64" not installed for action "Removed".
# echo $?
0

@jan-kolarik
Copy link
Member Author

@pkratoch Thank you, I understand now 🙂 I will prepare the additional test for that.

Hm, I think it works, there is just nothing to do... or did I misunderstand what you meant?

You're right, I've just seen the Warning there and overlooked that the whole operation exited successfully.

@jan-kolarik jan-kolarik force-pushed the jkolarik/group-upgrade-rollback-2-test branch from 029b418 to f13012d Compare September 14, 2022 05:12
@jan-kolarik jan-kolarik force-pushed the jkolarik/group-upgrade-rollback-2-test branch from f13012d to 0829f2e Compare September 14, 2022 05:13
@pkratoch pkratoch merged commit 541e6cd into master Sep 21, 2022
@jan-kolarik jan-kolarik deleted the jkolarik/group-upgrade-rollback-2-test branch December 7, 2022 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants