Skip to content

Fix/bootstrap starting for def #1270

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

Merged
merged 12 commits into from
Oct 23, 2018

Conversation

elukoyanov
Copy link
Collaborator

Removed config's bootstrap and teardown calls for codeceptjs def and codeceptjs list

Before, bootstrap has started (and set up connection to browserstack, for example) before def command, which is unnecessary.

Also, teardown has started after these commands. TypeError: done is not a function error occurred.

Copy link
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

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

Ok, I have 2 suggestions:

  1. make init synchronous without callback
  2. add bootstrap execution to run-multiple command

lib/codecept.js Outdated
*/
bootstrap(done) {
runHooks(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like runHooks are synchronous now, so done is not needed and callback functions.
Also I'm going to deprecate custom hooks in config so this code should be 100% synchronous

Copy link
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

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

Ok, I have 2 suggestions:

  1. make init synchronous without callback
  2. add bootstrap execution to run-multiple command
  3. remove bootstrap from gherkin* commands (as they don't execute tests)

}
if (err) throw new Error(`Error while running init :${err}`);

codecept.runBootstrap((err) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

bootstrap should not be run for listing steps definition (as it's not executed for def)

files.push(fsPath.resolve(file));
});
output.print(`Loaded ${files.length} files`);
codecept.runBootstrap((err) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

gherkin:snippets should not execute bootstrap (as it's omitted for def/list commands as well)

@codeceptjs codeceptjs deleted a comment Oct 23, 2018
@elukoyanov
Copy link
Collaborator Author

Done 1st and 3rd.

About 2nd:
bootstrap runs for every browser config in multiple section.
It should not be called for run-multiple.
We have bootstrapAll/teardownAll for multiple hook

@codeceptjs codeceptjs deleted a comment Oct 23, 2018
@DavertMik
Copy link
Contributor

Awesome, thanks!

@DavertMik DavertMik merged commit e1263f6 into codeceptjs:master Oct 23, 2018
@elukoyanov elukoyanov deleted the Fix/bootstrap-starting-for-def branch February 20, 2019 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants