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

Migrate to forked Cucumber 0.10 #3

Merged
merged 13 commits into from
Sep 23, 2021
Merged

Migrate to forked Cucumber 0.10 #3

merged 13 commits into from
Sep 23, 2021

Conversation

ilslv
Copy link
Contributor

@ilslv ilslv commented Sep 16, 2021

Synopsis

Upgrade to a new version of cucumber_rust.

Solution

Introduce concurrent tests runner.

Checklist

  • Created PR:
    • In draft mode
    • Name contains Draft: prefix
    • Name contains issue reference
    • Has k:: labels applied
    • Has assignee
  • Documentation is updated (if required)
  • Tests are updated (if required)
  • Changes conform code style
  • CHANGELOG entry is added (if required)
  • FCM (final commit message) is posted
    • and approved
  • Review is completed and changes are approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • Draft: prefix is removed
    • All temporary labels are removed

@ilslv ilslv added the k::toolchain Related to project toolchain label Sep 16, 2021
@ilslv ilslv self-assigned this Sep 16, 2021
@ilslv
Copy link
Contributor Author

ilslv commented Sep 17, 2021

@tyranron there is a problem, while running concurrent tests for firefox, because geckodriver doesn't support concurrent sessions: mozilla/geckodriver#1523 (comment), jonhoo/fantoccini#111 (comment). While chrome works just fine.

There already was an effort to hack around this: jonhoo/fantoccini#100. We can try merging this (seems like pretty straightforward task) or just run tests as @serial in case of firefox.

@ilslv
Copy link
Contributor Author

ilslv commented Sep 20, 2021

@tyranron we'll have to fallback to serial scenario execution, because even if we'll have multiple connections to single WebDriver client, we won't get isolation. This means, if one connection will redirect to foo.com and later second connection will redirect to bar.com, first connection will work with bar.com too.

@tyranron
Copy link
Member

@ilslv can we enable serial execution just for Firefox and leave it concurrent for Chrome?

@ilslv
Copy link
Contributor Author

ilslv commented Sep 20, 2021

@tyranron yep, already done that

@tyranron
Copy link
Member

@ilslv ok, that will do then! Just leave a fat comment explaining why that is done and we're ready for review, I guess.

@ilslv
Copy link
Contributor Author

ilslv commented Sep 21, 2021

FCM

Migrate to forked Cucumber 0.10 (#3)

@ilslv ilslv added the enhancement Improvement of existing features or bugfix label Sep 21, 2021
@ilslv ilslv requested a review from tyranron September 21, 2021 13:44
@tyranron tyranron added k::testing Related to testing and/or automated tests k::toolchain Related to project toolchain and removed k::toolchain Related to project toolchain labels Sep 21, 2021
@tyranron tyranron changed the title Draft: Upgrade cucumber_rust Upgrade cucumber_rust for concurrent execution Sep 21, 2021
@tyranron tyranron marked this pull request as ready for review September 21, 2021 21:20
@ilslv ilslv changed the title Upgrade cucumber_rust for concurrent execution Migrate to forked Cucumber 0.10 Sep 22, 2021
@ilslv
Copy link
Contributor Author

ilslv commented Sep 22, 2021

@tyranron I've decreased max_concurrent_scenarios to 5 to avoid hyper IncompleteMessage and CannelClosed errors. Locally I've been running with debug=no and 10 max_concurrent_scenarios just fine and got confused, once haven't find this option on CI. Is there a reason CI tests are running in debug?

@tyranron
Copy link
Member

@ilslv no, I think it has sense to run CI in release mode.

@ilslv ilslv merged commit 1148f78 into master Sep 23, 2021
@ilslv ilslv deleted the upgrade-cucumber branch September 23, 2021 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::testing Related to testing and/or automated tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants