Skip to content

Commit

Permalink
Improve warning messages for pre run validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@otsch
otsch committed Sep 14, 2024
1 parent f5d5de1 commit 9987c41
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed
* The `maxOutputs()` method is now also available and working on `Group` steps.
* Improved warning messages for step validations that are happening before running a crawler.

## [1.10.0] - 2024-08-05
### Added
Expand Down
66 changes: 54 additions & 12 deletions src/Steps/BaseStep.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,11 @@ public function validateBeforeRun(BaseStep|array $previousStepOrInitialInputs):
'instead of keep()',
);
} elseif ($outputType === StepOutputType::Mixed) {
$stepClassName = get_class($this);

$this->logger?->warning(
'The ' . $stepClassName . ' step potentially yields scalar value outputs (= single ' .
'string/int/bool/float with no key like in an associative array or object). If it does (yield a ' .
'scalar value output), it can not keep that output value, because it needs a key for that. ' .
'To avoid this, define a key for scalar outputs by using the keepAs() method.',
$this->getPreValidationRunMessageStartWithStepClassName() . ' potentially yields scalar value ' .
'outputs (= single string/int/bool/float with no key like in an associative array or object). ' .
'If it does (yield a scalar value output), it can not keep that output value, because it needs ' .
'a key for that. To avoid this, define a key for scalar outputs by using the keepAs() method.',
);
}
}
Expand All @@ -332,13 +330,12 @@ public function validateBeforeRun(BaseStep|array $previousStepOrInitialInputs):
'array or object). Please define a key for the input data to keep, by using keepAs() instead.',
);
} elseif ($previousStepOutputType === StepOutputType::Mixed) {
$stepClassName = get_class($this);

$this->logger?->warning(
'The step before the ' . $stepClassName . ' step, potentially yields scalar value outputs ' .
'(= single string/int/bool/float with no key like in an associative array or object). If it does ' .
'(yield a scalar value output) the next step can not keep it by using keepFromInput(). To avoid ' .
'this, define a key for scalar inputs by using the keepInputAs() method.',
$this->getPreValidationRunMessageStartWithStepClassName($previousStepOrInitialInputs) .
' potentially yields scalar value outputs (= single string/int/bool/float with no key like in ' .
'an associative array or object). If it does (yield a scalar value output) the next step can not ' .
'keep it by using keepFromInput(). To avoid this, define a key for scalar inputs by using the ' .
'keepInputAs() method.',
);
}
}
Expand Down Expand Up @@ -440,6 +437,51 @@ protected function validateFirstStepBeforeRun(array $initialInputs): void
}
}

protected function getPreValidationRunMessageStartWithStepClassName(?BaseStep $step = null): string
{
$stepClassName = $this->getStepClassName($step);

if ($stepClassName) {
return 'The ' . $stepClassName . ' step';
} else {
$stepClassName = $this->getParentStepClassName($step);

if (
$stepClassName &&
$stepClassName !== 'Crwlr\\Crawler\\Steps\\Step' &&
$stepClassName !== 'Crwlr\\Crawler\\Steps\\BaseStep'
) {
return 'An anonymous class step, that is extending the ' . $stepClassName . ' step';
} else {
return 'An anonymous class step';
}
}
}

protected function getStepClassName(?BaseStep $step = null): ?string
{
$stepClassName = get_class($step ?? $this);

if (str_contains($stepClassName, '@anonymous')) {
return null;
}

return $stepClassName;
}

protected function getParentStepClassName(?BaseStep $step = null): ?string
{
$parents = class_parents($step ?? $this);

$firstLevelParent = reset($parents);

if ($firstLevelParent && is_string($firstLevelParent) && !str_contains($firstLevelParent, '@anonymous')) {
return $firstLevelParent;
}

return null;
}

protected function getInputKeyToUse(Input $input): ?Input
{
if ($this->useInputKey !== null) {
Expand Down
157 changes: 157 additions & 0 deletions tests/Steps/BaseStepTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Crwlr\Crawler\Steps\Loading\Http;
use Crwlr\Crawler\Steps\Step;
use Crwlr\Crawler\Steps\StepOutputType;
use Exception;
use Generator;
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Psr7\Response;
Expand Down Expand Up @@ -235,6 +236,74 @@ protected function invoke(mixed $input): Generator
},
);

test(
'the warning message, when output type is mixed and keep() was used but not keepAs() with an anonymous step ' .
'class, extending a step that isn\'t one of the abstract classes Step or BaseStep, contains the parent step ' .
'class',
function () {
class ParentStepClass extends Step
{
protected function invoke(mixed $input): Generator
{
yield $input;
}
}

$step = new class extends ParentStepClass {};

$step->addLogger(new CliLogger())->keep()->validateBeforeRun(Http::get());

expect($this->getActualOutputForAssertion())
->toContain(
'An anonymous class step, that is extending the tests\\Steps\\ParentStepClass step potentially ' .
'yields scalar value outputs',
);
},
);

test(
'the warning message, when output type is mixed and keep() was used but not keepAs() with an anonymous step ' .
'class, extending one of the abstract classes Step or BaseStep, only mentions that it is an anonymous step class',
function (string $extendClass) {
$step = null;

if ($extendClass === Step::class) {
$step = new class extends Step {
protected function invoke(mixed $input): Generator
{
yield $input;
}
};
} elseif ($extendClass === BaseStep::class) {
$step = new class extends BaseStep {
protected function invoke(mixed $input): Generator
{
yield $input;
}

public function invokeStep(Input $input): Generator
{
yield from $this->invoke($input);
}
};
}

if ($step === null) {
throw new Exception('Invalid $extendClass parameter');
}

$step->addLogger(new CliLogger())->keep()->validateBeforeRun(Http::get());

expect($this->getActualOutputForAssertion())
->toContain(
'An anonymous class step potentially yields scalar value outputs',
);
},
)->with([
[Step::class],
[BaseStep::class],
]);

it('does not throw an exception or log a warning when output type is scalar and keepAs() was called', function () {
helper_getInputReturningStep()->addLogger(new CliLogger())->keepAs('foo')->validateBeforeRun(Http::get());

Expand Down Expand Up @@ -313,6 +382,94 @@ public function outputType(): StepOutputType
->toContain('the next step can not keep it by using keepFromInput()');
});

test(
'the warning message, when keepFromInput() was called and previous step yields mixed outputs with an anonymous ' .
'step class, extending a step that isn\'t one of the abstract classes Step or BaseStep, contains the parent step ' .
'class',
function () {
class ParentStepClassTwo extends Step
{
protected function invoke(mixed $input): Generator
{
yield 'yo';
}

public function outputType(): StepOutputType
{
return StepOutputType::Mixed;
}
}

$stepWithMixedOutputType = new class extends ParentStepClassTwo {};

Http::get()
->keepFromInput()
->addLogger(new CliLogger())
->validateBeforeRun($stepWithMixedOutputType);

expect($this->getActualOutputForAssertion())
->toContain(
'An anonymous class step, that is extending the tests\\Steps\\ParentStepClassTwo step potentially ' .
'yields scalar value outputs',
);
},
);

test(
'the warning message, when keepFromInput() was called and previous step yields mixed outputs with an anonymous ' .
'step class, extending one of the abstract classes Step or BaseStep, only mentions that it is an anonymous step ' .
'class',
function (string $extendClass) {
$stepWithMixedOutputType = null;

if ($extendClass === Step::class) {
$stepWithMixedOutputType = new class extends Step {
protected function invoke(mixed $input): Generator
{
yield 'yo';
}

public function outputType(): StepOutputType
{
return StepOutputType::Mixed;
}
};
} elseif ($extendClass === BaseStep::class) {
$stepWithMixedOutputType = new class extends BaseStep {
protected function invoke(mixed $input): Generator
{
yield 'yo';
}

public function outputType(): StepOutputType
{
return StepOutputType::Mixed;
}

public function invokeStep(Input $input): Generator
{
yield from $this->invoke($input);
}
};
}

if ($stepWithMixedOutputType === null) {
throw new Exception('Invalid $extendClass parameter');
}

Http::get()
->keepFromInput()
->addLogger(new CliLogger())
->validateBeforeRun($stepWithMixedOutputType);

expect($this->getActualOutputForAssertion())
->toContain('An anonymous class step potentially yields scalar value outputs');
},
)->with([
[Step::class],
[BaseStep::class],
]);

/* ----------------------------- keep() ----------------------------- */

it('adds all from array output to the keep array in the output object, when keep() is called', function () {
Expand Down

0 comments on commit 9987c41

Please sign in to comment.