You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
These changes below could not be separated into their own PRs, as fixing the tests breaks the operator's CI/CD pipeline (the code is broken in master now), and fixing the issue is not provable (the tests are false-positive in master now). So, they go as one PR with two topics.
Part 1:
While adding the type-annotations in #200, some data structures were split and their handling was reimplemented slightly. In particular, full body and body essence were separated -- to be used in different places and to make it clear that they have different content.
By intention, the essence is a deep copy of the body (there is a line to do that).
However, in the body's essence calculation, labels and annotations were taken from the original body as is, as the dict objects, and the annotations were modified in place to remove the system's/framework's ones -- which broke the intention of the essence being a deep copy of the body.
As a result, the important annotations of the last-seen state were lost also from the body (not only from the essence), and the framework could not detect the cause properly in case of the multi-handler operators (with at least 2 handling cycles), so it always assumed that this is a newly created object (due to absence of the last-seen state), and repeated the handling non-stop, again and again.
The infinite handling during 2 seconds of the e2e tests was never checked and asserted, so it went unnoticed, despite it was logged and captured.
This case is now prevented from happening in the future (beside the fix) by checking how many times every handler-of-interest has succeeded/failed.
Part 2:
While fixing the above-mentioned issue, it was found that the e2e tests were working just by luck: they took the global registry of the previously executed tests, added new handlers, and for this reason could find all the needed log lines in the output -- they were coming from the previous tests, not from the current one.
This issue is now fixed by moving the clear_default_registry fixture to the top-level conftest plugin, and is now auto-applied for all the tests (both unit and e2e).
Once this "test overlapping" effect was removed, few e2e tests actually broke for valid reasons. So, they had to be fixed with proper timing.
Types of Changes
Bugfix
The text was updated successfully, but these errors were encountered:
Fix the never-ending handling cycle, and fix the tests to catch this.
Description
These changes below could not be separated into their own PRs, as fixing the tests breaks the operator's CI/CD pipeline (the code is broken in master now), and fixing the issue is not provable (the tests are false-positive in master now). So, they go as one PR with two topics.
Part 1:
While adding the type-annotations in #200, some data structures were split and their handling was reimplemented slightly. In particular, full body and body essence were separated -- to be used in different places and to make it clear that they have different content.
By intention, the essence is a deep copy of the body (there is a line to do that).
However, in the body's essence calculation, labels and annotations were taken from the original body as is, as the
dict
objects, and the annotations were modified in place to remove the system's/framework's ones -- which broke the intention of the essence being a deep copy of the body.As a result, the important annotations of the last-seen state were lost also from the body (not only from the essence), and the framework could not detect the cause properly in case of the multi-handler operators (with at least 2 handling cycles), so it always assumed that this is a newly created object (due to absence of the last-seen state), and repeated the handling non-stop, again and again.
The infinite handling during 2 seconds of the e2e tests was never checked and asserted, so it went unnoticed, despite it was logged and captured.
This case is now prevented from happening in the future (beside the fix) by checking how many times every handler-of-interest has succeeded/failed.
Part 2:
While fixing the above-mentioned issue, it was found that the e2e tests were working just by luck: they took the global registry of the previously executed tests, added new handlers, and for this reason could find all the needed log lines in the output -- they were coming from the previous tests, not from the current one.
This issue is now fixed by moving the
clear_default_registry
fixture to the top-level conftest plugin, and is now auto-applied for all the tests (both unit and e2e).Once this "test overlapping" effect was removed, few e2e tests actually broke for valid reasons. So, they had to be fixed with proper timing.
Types of Changes
The text was updated successfully, but these errors were encountered: