Skip to content
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.

Sign up for GitHub

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

Improve warning messages for pre run validation #164

Merged
merged 1 commit into from
Sep 14, 2024
Merged

Improve warning messages for pre run validation #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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