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.

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

fix(cli): allow beforeafter validation schema to run engines without flow #2396

Conversation

bernardobridge
Copy link
Contributor

@bernardobridge bernardobridge commented Jan 4, 2024

Description

In theory any engine can run as a before/after hook. However, if the engine doesn't include flow, it will currently fail. This PR fixes that, and also introduces an error message when before.engine is configured without the corresponding config.engine (similar to #2101).

Having this will enable e.g. playwright scenarios to be run as a before.

Note: I will fix the VS code schema separately in another PR.

Pre-merge checklist

  • Does this require an update to the docs? Yes, maybe clarify usage of before/after with other engines
  • Does this require a changelog entry? Yes

@bernardobridge bernardobridge changed the title Bernardobridge/art 1547 allow beforeafter validate script schema to run arbitrary fix(cli): allow beforeafter validate script schema to run arbitrary Jan 4, 2024
@bernardobridge bernardobridge requested a review from a team January 4, 2024 18:56
@bernardobridge bernardobridge changed the title fix(cli): allow beforeafter validate script schema to run arbitrary fix(cli): allow beforeafter validatation schema to run engines without flow Jan 4, 2024
@bernardobridge bernardobridge changed the title fix(cli): allow beforeafter validatation schema to run engines without flow fix(cli): allow beforeafter validation schema to run engines without flow Jan 5, 2024
@@ -105,9 +105,13 @@ const scenarioItem = Joi.object({
});

const beforeAfterSchema = Joi.object({
Copy link
Contributor Author

@bernardobridge bernardobridge Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the schema would look at config.engines to determine this. However, this is more complicated than just looking at the engine field within before/after itself:

  • If it doesn't have it, it's http (default), so it needs flow.
  • If it has it, and if it's socketio, ws or http, it needs flow. Otherwise it does not require flow (e.g. custom engine or playwright).

Looking at config.engines doesn't actually make sense, since you could be putting a config there for an engine you're using in your scenario, but not in before/after.

Comment on lines +496 to +501

if (typeof engine === 'undefined') {
throw new Error(
`Failed to run ${hook} hook: unknown engine "${name}". Did you forget to include it in "config.engines.${name}"?`
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar logic to #2101 for consistency

@bernardobridge bernardobridge merged commit 21c8e17 into main Jan 12, 2024
18 checks passed
@bernardobridge bernardobridge deleted the bernardobridge/art-1547-allow-beforeafter-validate-script-schema-to-run-arbitrary branch January 12, 2024 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant