-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fail measurements if any of the jobs returned issues #389
Conversation
dcfc3c5
to
e0543e5
Compare
This property indicates if the Measurement finished ok or if there were any errors reported from the individual measurement test tool jobs that invalidate the results. The results can still be evaluated but they should be considered as invalid for some use cases e.g. setting new baselines. Signed-off-by: Ondrej Lichtner <[email protected]>
This is equivalent to the measurement_success so this is no longer needed. Signed-off-by: Ondrej Lichtner <[email protected]>
e0543e5
to
9703145
Compare
The measurement_success property is now exposed as the result value of the MeasurementResult object. Signed-off-by: Ondrej Lichtner <[email protected]>
Not using **kwargs means that it wasn't possible to use named init arguments when creating a new instance of the object. Signed-off-by: Ondrej Lichtner <[email protected]>
9703145
to
aadca4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Briefly discussed offline, as a hotfix to get rid of 0 measurements messing with rolling baselines this is good enough.
However moving forward, it would be better to either move some functionality of measurements to results or vice versa.
The 'result' name conflicts with the 'result' from the main for loop, this overrides the value of the 'result' name which is used a few lines lower to calculate measurement_success. Signed-off-by: Ondrej Lichtner <[email protected]>
Looks good. |
For some reason this Aggregated class was missing the individual_results property. Signed-off-by: Ondrej Lichtner <[email protected]>
b6f7efb
to
012921b
Compare
testing done internally in beaker job i believe this is ready to be merged. |
Ack to merge. |
Description
This is necessary to detect situation when something went wrong with the measurement and we should disqualify the measurement results from being fully trustworthy, e.g. for setting baselines.
Tests
Needs to be tested for all types of measurements, ideally with failures in the measurement process to see the changed results.
Reviews
@Axonis @enhaut @jtluka
Closes: #