Calibration Stage Fix for crashing testcases #3557
Conversation
| // todo | ||
| // remove this guy from corpus queue | ||
| let retry = RetryCountRestartHelper::no_retry(state, &self.name)?; | ||
| if !retry { |
There was a problem hiding this comment.
This is the main change
There was a problem hiding this comment.
what happens for the stages after CalibrationStage?
There was a problem hiding this comment.
or did you try running this?
I suspect that stages after this will panic because if you remove the corpus during the 1st stage, then the subsequent stage will still look at the same testcase, which no more exists
There was a problem hiding this comment.
imagine you have let stages = tuple_list!(calibration, power);
you can disable this during the first calibration.
but the second power stage is still executed.
there're 2 problems.
1st is that when you execute power stage, testcase is gone.
2nd is that this doesn't actually solve the problem unless the execution of power stage is somehow cancelled. (if it is normally executed, it still looks for the metadata that is not there)
There was a problem hiding this comment.
I can throw an error from here and handle it somewhere higher up?
| log::info!("Disabling crashing/timeouting testcase {id} during calibration"); | ||
| let insert_result = state | ||
| .current_testcase_mut()? | ||
| .try_add_metadata(DisabledInCalibrationStageMetadata); |
There was a problem hiding this comment.
what is this metadata used for?
There was a problem hiding this comment.
Documentation why the testcase is disabled
There was a problem hiding this comment.
but this metadata is never used
There was a problem hiding this comment.
It's used below?
Also it'll be on disk so the end user can see what's up
There was a problem hiding this comment.
Handling the Err(err) case implicitly uses the metadata
Description
This fixes a todo and disables flakey testcases that somehow got into the corpus.
Checklist
./scripts/precommit.shand addressed all comments