This repository has been archived by the owner on Sep 5, 2024. It is now read-only.
Conversion Host Settings - Unit tests for helpers, prevent corner case of reappearing failures #928
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of the Conversion Hosts UI feature BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1693339
As part of #884, I started filling in the gaps in unit testing of the conversion host settings UI code. I started with the helpers that handle generating the combined list items from parsed and indexed task and conversion host objects, on a hunch that there was a corner case I had missed, and I was right:
If you checkout ce342e9 and try to run the new tests, you'll see two failures due to the
{ ...exampleTask(20, 'enable', '11'), ...failed }
task appearing as an active enablement task even though there were successful enable and disable tasks after it. This means if a user had attempted to enable VM 11 and it failed, then attempted again and it succeeded, then removed it and it succeeded, when the removal completed it would show "enable failed" again instead of disappearing from the list.This bug is due to the fact that the
getActiveConversionHostEnableTasks
performed these filters in the wrong order:This was fine in the case of a failed enablement that had been retried, because the first filter would include the failure, but the second filter would remove it since it matched a configured host. But as soon as that host is removed, the second filter wouldn't remove the failure anymore, and we get the bug.
60e6246 fixes this bug by simply reversing the order of these two filters:
This way, only the latest task for each resource can be shown in the list, regardless of its state. With the filters reversed, all tasks for VM 11 are filtered out, because the latest task is a disable task, and when the disable completes, that resource is (correctly) no longer present in the list.