Fix installing in fresh Rails app with Stimulus

solidusio / solidus_starter_frontend

🎨 Rails-based starter kit for your Solidus storefront.

BSD 3-Clause "New" or "Revised" License

58 stars 40 forks source link

Fix installing in fresh Rails app with Stimulus #371

Closed tvdeyen closed 2 months ago

tvdeyen commented 2 months ago

Summary

Fresh Rails apps with Stimulus installed already have a app/javascript/controller/index.js file with boilerplate code. Since we install this file as well we need to overwrite it if auto accept is enabled.

Example of failing build in Solidus core https://app.circleci.com/pipelines/github/solidusio/solidus/6567/workflows/570ef440-07cb-470a-bb91-9f08ba79c1de/jobs/60769

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

[x] I have written a thorough PR description.
[x] I have kept my commits small and atomic.
[x] I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

fredericDelaporte commented 2 months ago

About the commit message referencing an issue I have opened elsewhere:

Since Chrome 128 there is a Search Engine selection popup even in headless mode that needs to be accepted. This leads to Capybara Element not found exceptions during system specs.

That is a bit more convoluted.

The actual change would be the headless argument: with Chrome 127, it was a shortcut for headless=old. It seems with Chrome 128 to behave as headless=new. See this Chrome blog for details about these two modes.

The search engine choice screen seems to be a thing since at least version 127, but only with the "new" headless mode.

Your code seems to use headless without a value, so, defaulting to "old" for older Chrome browsers, susceptible to default to "new" for newer Chrome browsers as explained in the blog previously linked.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.97%. Comparing base (8e003e7) to head (c368c16). Report is 5 commits behind head on main.

Files	Patch %	Lines
.../spec/support/solidus_starter_frontend/capybara.rb	66.66%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #371 +/- ## ========================================== - Coverage 95.99% 95.97% -0.03% ========================================== Files 207 207 Lines 4874 4869 -5 ========================================== - Hits 4679 4673 -6 - Misses 195 196 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tvdeyen commented 2 months ago

Thanks @fredericDelaporte since the :headless_chrome setup is handled in Rails itself https://github.com/rails/rails/blob/6f57590388ca38ed2b83bc1207a8be13a9ba2aef/actionpack/lib/action_dispatch/system_testing/browser.rb#L61 we are fixing it like this for now. After a Rails update we can probably remove this.

Thanks for the heads up and explanation of the actual cause of this issue. Very much appreciated 🙏🏻

tvdeyen commented 2 months ago

💚 All backports created successfully

Status	Branch	Result
✅	v4.3

Questions ?

Please refer to the Backport tool documentation