Make optional @screenshot annotations in step definition methods

[ricardorlg-aval]

Currently allowing @screenshot annotations in step definition methods, causes that if no annotation is present, serenity uses a default strategy screenshot, of before each, But if I have defined only for failures, Serenity is ignoring that in the steps definitions.

I propose to make optional the support of the annotation, for example a new property like, ENABLE_SCREENSHOT_ANNOTATION_IN_STEPS, and if set, use it otherwise use the old behavior.

[ricardorlg-aval]

@wakaleo ⬆️

[wakaleo]

Hmm, shouldn’t the fallback behaviour just use the value configured in Serenity properties? Introducing a new property for this would be a bit counter-intuitive to use

[ricardorlg-aval]

shouldn’t the fallback behavior just use the value configured in Serenity properties? It should, but currently when no annotation is found it sets the screenshot preference to TakeScreenshots.UNDEFINED and then Serenity will delegate the Screenshoot permission to the deprecated legacyScreenshotConfiguration(takeScreenshots)

[ricardorlg-aval]

Maybe in StepDefinitionAnnotationReaderclass, if non annotation is found, we should try to get the screenshot preference from properties if exists otherwise use the UNDEFINED.

[wakaleo]

Agreed. If you are in the code at the moment could you propose a PR to do this?

[ricardorlg-aval]

done, please review it.