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.

Sign up for GitHub

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

Jump to bottom

Bundle update, switch integration test drivers #681

Merged
merged 4 commits into from
Oct 28, 2019
Merged

Bundle update, switch integration test drivers #681

merged 4 commits into from
Oct 28, 2019

Conversation

wvengen
Copy link
Member

@wvengen wvengen commented Oct 25, 2019 •
edited
Loading

  • bundle update (important updates)
  • get rid of warnings because of new conventions (like factory_bot)
  • switch to apparition integration test js driver (no webdriver install necessary) (the headless webkit option used became a dead end)
  • silence mail logs during testing

TO DO:

  • bump required Ruby version, in docs, CI, docker (Capybara requirement)
  • update development docs (no webkit but chromium for integration tests)
  • make tests work in Docker development setup

There is still the occasional integration test failure, but that is still caught by rspec-rerun on CI (and we already had those).

@wvengen wvengen added this to the 4.7 milestone Oct 25, 2019
@wvengen wvengen added needs work tech debt technical debt labels Oct 25, 2019
@wvengen wvengen force-pushed the feature/bundle-update branch 2 times, most recently from d3fc488 to d4e8e8c Compare October 26, 2019 13:14
@paroga
Copy link
Member

paroga commented Oct 27, 2019

nice update! can you create separate changes? IMHO the faker update, the test driver switch and the bundle update could all be done in independent changes.
ad puma: i didn't test your change, but i had some troubles when enabling it on my Docker instance. I had to change the Procfile to get it running

@wvengen
Copy link
Member Author

wvengen commented Oct 28, 2019

Thanks for checking! Ouch, thanks for pointing out the puma change (the production container still would have worked, because it wouldn't have installed puma, but nonetheless).
I'll push a new version with separated commits shortly.

paroga
paroga approved these changes Oct 28, 2019
View reviewed changes
Gemfile.lock Show resolved Hide resolved
spec/integration/balancing_spec.rb
@@ -167,8 +172,10 @@
click_link I18n.t('finance.balancing.edit_results_by_articles.add_article')
expect(page).to have_selector('form#new_order_article')
within('#new_order_article') do
select new_article.name, :from => 'order_article_article_id'
find('#order_article_article_id').select(new_article.name)
sleep 0.1
Copy link
Member

Choose a reason for hiding this comment

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

i don't think that i need to mention that sleep looks wrong 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for mentioning, I'd like to look into this sometime
https://github.com/twalpole/apparition#timing-problems
https://github.com/teamcapybara/capybara#asynchronous-javascript-ajax-and-friends

@wvengen wvengen merged commit 6d9c35d into master Oct 28, 2019
@wvengen wvengen deleted the feature/bundle-update branch October 28, 2019 17:58
@wvengen wvengen mentioned this pull request Oct 29, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paroga paroga paroga approved these changes

Assignees
No one assigned
Labels
tech debt technical debt
Projects
None yet
Milestone
4.7
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wvengen @paroga