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

Step definitions do not support callback style after being sychronized by serenity-js #8

Closed
InvictusMB opened this Issue Dec 1, 2016 · 2 comments

Comments

2 participants
@InvictusMB
Contributor

InvictusMB commented Dec 1, 2016

If you do Getting WebDriver and Cucumber in sync step from tutorial before anything else then the test suite starts failing on Given step.

The worst part is that in console you don't see any output from test at all unless you disable preprotractor npm task. Probably not relevant to this issue, but preprotractor npm task just doesn't end properly on Mac OS.

I have created a branch to reproduce the issue
https://github.com/InvictusMB/serenity-js-getting-started/tree/syncIssue

Console output

➜  npm test

> serenity-js-getting-started@1.0.0 test /serenity-js-getting-started
> npm run protractor


> serenity-js-getting-started@1.0.0 protractor /serenity-js-getting-started
> protractor ./protractor.conf.js --silent

[11:11:37] I/local - Starting selenium standalone server...
[11:11:37] I/launcher - Running 1 instances of WebDriver
[11:11:38] I/local - Selenium standalone server started at http://192.168.0.103:56557/wd/hub
Feature: Add new items to the todo list

    In order to avoid having to remember things that need doing
    As a forgetful person
    I want to be able to record what I need to do in a place where I won't forget about them

cucumber event handlers attached via registerHandler are now passed the associated object instead of an event
getPayloadItem will be removed in the next major release
  Scenario: Adding an item to a list with other items
  ✖ Given that James has a todo list containing Buy some cookies, Walk the dog
  - When he adds Buy some cereal to his list
  - Then his todo list should contain Buy some cookies, Walk the dog, Buy some cereal

Failures:

1) Scenario: Adding an item to a list with other items - features/add_new_items.feature:7
   Step: Given that James has a todo list containing Buy some cookies, Walk the dog - features/add_new_items.feature:8
   Step Definition: node_modules/serenity-js/lib/serenity-cucumber/src/serenity-cucumber/webdriver_synchroniser.ts:44
   Message:
     function accepts a callback and returns a promise

1 scenario (1 failed)
3 steps (1 failed, 2 skipped)
0m00.001s
[11:11:40] I/local - Shutting down selenium standalone server.
[11:11:40] I/launcher - 0 instance(s) of WebDriver still running
[11:11:40] I/launcher - chrome #01 failed 1 test(s)
[11:11:40] I/launcher - overall: 1 failed spec(s)
[11:11:40] E/launcher - Process exited with error code 1

I believe this mismatch is caused by synchronizing wrapper that always returns a promise even though a wrapped function may accept a callback thus the arity of a created wrapper should be different. But I cannot reason why only the first step fails. Because others are skipped if the first one is pending or failed. Otherwise they would fail in this case too.

@InvictusMB InvictusMB changed the title from Step definition for Given does not support callback style after being sychronized by serenity-js to Step definitions do not support callback style after being sychronized by serenity-js Dec 1, 2016

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Dec 1, 2016

Owner

Hey @InvictusMB, you're correct, the synchroniser does not support the mixed mode (callbacks and promises), assuming that you'd only ever use promises when working with Serenity/JS.
This assumption feels overly optimistic since people might need to transition an existing code base to Serenity/JS, in which case it's perfectly valid for them to have both callbacks and promises in one test suite.

I'll look into adding support for callback-style step definitions, and am happy to review pull requests too if you have some time you'd be willing to contribute?


The preprotractor step you mention invokes the webdriver-manager module and downloads the selenium server and the chrome driver from google's CDN. This can sometimes fail due to issues with proxies, or a previous download that didn't finish correctly. If you have a look in the node_modules/protractor/node_modules/webdriver-manager/selenium you should see the following files:

ls -lah ~/todomvc-protractor-cucumber/node_modules/protractor/node_modules/webdriver-manager/selenium/ 

total 80336
drwxr-xr-x+  7 jan  staff   238B 12 Nov 17:55 .
drwxr-xr-x+ 15 jan  staff   510B 12 Nov 17:55 ..
-rwxr-xr-x+  1 jan  staff    10M 12 Nov 17:55 chromedriver_2.25
-rw-r--r--+  1 jan  staff   4.4M 12 Nov 17:55 chromedriver_2.25mac64.zip
-rwxr-xr-x+  1 jan  staff   3.2M 10 Oct 12:53 geckodriver-v0.11.1
-rw-r--r--+  1 jan  staff   1.2M 12 Nov 17:55 geckodriver-v0.11.1-macos.tar.gz
-rw-r--r--+  1 jan  staff    20M 12 Nov 17:55 selenium-server-standalone-2.53.1.jar

If you don't, or they're if a different size, remove the selenium directory and re-run the preprotractor step.

Please let me know if that helped?

Best,
Jan

Owner

jan-molak commented Dec 1, 2016

Hey @InvictusMB, you're correct, the synchroniser does not support the mixed mode (callbacks and promises), assuming that you'd only ever use promises when working with Serenity/JS.
This assumption feels overly optimistic since people might need to transition an existing code base to Serenity/JS, in which case it's perfectly valid for them to have both callbacks and promises in one test suite.

I'll look into adding support for callback-style step definitions, and am happy to review pull requests too if you have some time you'd be willing to contribute?


The preprotractor step you mention invokes the webdriver-manager module and downloads the selenium server and the chrome driver from google's CDN. This can sometimes fail due to issues with proxies, or a previous download that didn't finish correctly. If you have a look in the node_modules/protractor/node_modules/webdriver-manager/selenium you should see the following files:

ls -lah ~/todomvc-protractor-cucumber/node_modules/protractor/node_modules/webdriver-manager/selenium/ 

total 80336
drwxr-xr-x+  7 jan  staff   238B 12 Nov 17:55 .
drwxr-xr-x+ 15 jan  staff   510B 12 Nov 17:55 ..
-rwxr-xr-x+  1 jan  staff    10M 12 Nov 17:55 chromedriver_2.25
-rw-r--r--+  1 jan  staff   4.4M 12 Nov 17:55 chromedriver_2.25mac64.zip
-rwxr-xr-x+  1 jan  staff   3.2M 10 Oct 12:53 geckodriver-v0.11.1
-rw-r--r--+  1 jan  staff   1.2M 12 Nov 17:55 geckodriver-v0.11.1-macos.tar.gz
-rw-r--r--+  1 jan  staff    20M 12 Nov 17:55 selenium-server-standalone-2.53.1.jar

If you don't, or they're if a different size, remove the selenium directory and re-run the preprotractor step.

Please let me know if that helped?

Best,
Jan

@InvictusMB

This comment has been minimized.

Show comment
Hide comment
@InvictusMB

InvictusMB Dec 2, 2016

Contributor

@jan-molak I did reinstall protractor and updated webdriver manually but unfortunately that didn't help. Rebooting fixed it though :)

While digging into cucumber I've found that it detects step interface type by comparing the number of parameters accepted by step definition and number of parameters parsed. So the same check should be sufficient in wrapper.
However I also found that it supports es6 generator functions as step definitions as well. Therefore #9
Wrapping the generator is not as trivial as callback and requires pulling an extra dependency such as co. So it needs a separate discussion.

Contributor

InvictusMB commented Dec 2, 2016

@jan-molak I did reinstall protractor and updated webdriver manually but unfortunately that didn't help. Rebooting fixed it though :)

While digging into cucumber I've found that it detects step interface type by comparing the number of parameters accepted by step definition and number of parameters parsed. So the same check should be sufficient in wrapper.
However I also found that it supports es6 generator functions as step definitions as well. Therefore #9
Wrapping the generator is not as trivial as callback and requires pulling an extra dependency such as co. So it needs a separate discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment