Skip to content

Set Rails environment to test before invoking extension:test_app task#187

Closed
gsmendoza wants to merge 1 commit intomasterfrom
gsmendoza/eng-340-fix-soliduspaypalcommerceplatform-bundle
Closed

Set Rails environment to test before invoking extension:test_app task#187
gsmendoza wants to merge 1 commit intomasterfrom
gsmendoza/eng-340-fix-soliduspaypalcommerceplatform-bundle

Conversation

@gsmendoza
Copy link
Copy Markdown
Contributor

@gsmendoza gsmendoza commented Jun 8, 2022

This fixes the following error when running bundle exec rake in
SolidusPaypalCommercePlatform:

An error occurred in a `before(:suite)` hook.
Failure/Error: //= link spree/backend/all.js

Sprockets::FileNotFound:
  couldn't find file 'spree/backend/all.js'
  Checked in these paths:
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/spec/dummy/app/assets/config
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/spec/dummy/app/assets/images
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/spec/dummy/app/assets/stylesheets
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/app/assets/javascripts
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/app/assets/stylesheets

Cause

extension:specs, the default rake task of SolidusPaypalCommercePlatform,
executes the rake tasks and Ruby methods below in the given order:

The setup_assets method would only install the all.* frontend and backend
assets if either

  • Spree::Frontend and Spree::Backend are loaded, or
  • The Rails environment is test.

Currently, when we run bundle exec rake in SolidusPaypalCommercePlatform,
neither condition is met:

  • Spree::Frontend and Spree::Backend are NOT loaded.
  • The Rails environment is development.

Trigger

This bug surfaced after we applied solidusio/solidus#4251 (Fix: solidus:install adds the frontend
assets even if the repo does not have solidus_frontend).

Solution

Considering that the extension:test_app rake task generates the
spec/dummy directory, we can assume that it is expected to run under
test environment.

Testing

Checklist

  • I have structured the commits for clarity and conciseness.
  • I have added relevant automated tests for this change.

@gsmendoza gsmendoza self-assigned this Jun 8, 2022
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Jun 8, 2022

It looks like this PR is missing a label to determine the type of change it introduces. The maintainer should add one of the following labels:

  • bug for bugfixes.
  • enhancement for new features and improvements.
  • documentation for documentation changes.
  • security for security patches.
  • removed for feature removals.
  • infrastructure for internal changes that should not go in the changelog.

Additionally, the maintainer may also want to add one of the following:

  • breaking for breaking changes.
  • deprecated for feature deprecations.

Once the correct labels have been set, simply remove the needs changelog label label from this PR so I can merge it.

@mergify mergify Bot added the needs changelog label Needs a label to determine the type of change. label Jun 8, 2022
@@ -44,6 +44,7 @@ def install_test_app_task
end
Copy link
Copy Markdown
Contributor Author

@gsmendoza gsmendoza Jun 8, 2022

Choose a reason for hiding this comment

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

@waiting-for-dev In SolidusPaypalCommercePlatform's case, the extension:test_app wrapper task defined in lib/solidus_dev_support/rake_tasks.rb is invoked after Solidus::InstallGenerator runs. To recap, here are all the tasks and Ruby methods executed by bundle exec rake in SolidusPaypalCommercePlatform, including the extension:test_app wrapper task.

  1. The extension namespace defined in SolidusDevSupport::RakeTasks#install_test_app_task, which invokes
    the extension:test_app task.
  2. The Solidus extension:test_app task
  3. The common:test_app rake task in CommonRakeTasks#initialize
  4. Solidus::InstallGenerator
  5. The extension:test_app wrapper task in SolidusDevSupport (the test_app task above)
  6. The Solidus extension:test_app task again
  7. extension:specs, which is the default task of SolidusPaypalCommercePlatform

I'm guessing that the extension:test_app wrapper task (Step 5 above) is called as part of the whole invocation of the extension:test_app task. That invocation calls the "original" Solidus extension:test_app task first, before calling the overriding extension:test_app task from SolidusDevSupport.

Considering that Step 1 has already set up ENV['RAILS_ENV'] = 'test', there is no need to do the same for the extension:test_app wrapper task (Step 5 above), except to make it consistent with the change I made to Rake::Task['extension:test_app'].invoke below. Let me know if you prefer to err on the side of consistency.

Finaly, note that when the Solidus extension:test_app task is invoked a second time (in Step 6), it doesn't do anything. I still have to check but I'm guessing this is a normal behavior for Rake: it runs a task only once.

I'm making several guesses here concerning Rake, so I'll need to review Rake's documentation, especially with regard to how it handles overridden rake tasks.

@gsmendoza gsmendoza force-pushed the gsmendoza/eng-340-fix-soliduspaypalcommerceplatform-bundle branch from f62fd05 to 7e51b37 Compare June 8, 2022 09:49
@gsmendoza gsmendoza added bug Describes or fixes a bug. and removed needs changelog label Needs a label to determine the type of change. labels Jun 8, 2022
This fixes the following error when running `bundle exec rake` in
SolidusPaypalCommercePlatform:

```
An error occurred in a `before(:suite)` hook.
Failure/Error: //= link spree/backend/all.js

Sprockets::FileNotFound:
  couldn't find file 'spree/backend/all.js'
  Checked in these paths:
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/spec/dummy/app/assets/config
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/spec/dummy/app/assets/images
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/spec/dummy/app/assets/stylesheets
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/app/assets/javascripts
    /home/gsmendoza/repos/solidusio-contrib/solidus_paypal_commerce_platform/app/assets/stylesheets
```

Cause
-----

`extension:specs`, the default rake task of SolidusPaypalCommercePlatform,
executes the rake tasks and Ruby methods below in the given order:

* The `extension` namespace defined in
  `SolidusDevSupport::RakeTasks#install_test_app_task`. This namespace invokes
  the `extension:test_app` task
  (https://github.com/solidusio/solidus_dev_support/blob/e5fec9c039607ab9e97c3f821cc49f9bbec08796/lib/solidus_dev_support/rake_tasks.rb#L47).

* The Solidus `extension:test_app` task
  (https://github.com/solidusio/solidus/blob/07c88ddd1603699939ac0343965fa88dc2e1851a/core/lib/spree/testing_support/extension_rake.rb#L7)

* The `common:test_app` rake task in `CommonRakeTasks#initialize`
  (https://github.com/solidusio/solidus/blob/07c88ddd1603699939ac0343965fa88dc2e1851a/core/lib/spree/testing_support/common_rake.rb#L14)

* `Solidus::InstallGenerator#setup_assets`
  (https://github.com/solidusio/solidus/blob/07c88ddd1603699939ac0343965fa88dc2e1851a/core/lib/generators/solidus/install/install_generator.rb#L81)

The `setup_assets` method would only install the `all.*` frontend and backend
assets if either

* `Spree::Frontend` and `Spree::Backend` are loaded, or
* The Rails environment is test.

Currently, when we run `bundle exec rake` in SolidusPaypalCommercePlatform,
neither condition is met:

* `Spree::Frontend` and `Spree::Backend` are NOT loaded.
* The Rails environment is development.

Trigger
-------

This bug surfaced after we applied
solidusio/solidus#4251 (Fix: solidus:install
adds the frontend assets even if the repo does not have
`solidus_frontend`).

Solution
--------

Considering that the `extension:test_app` rake task generates the
`spec/dummy` directory, we can assume that it is expected to run under
test environment.
@gsmendoza gsmendoza force-pushed the gsmendoza/eng-340-fix-soliduspaypalcommerceplatform-bundle branch from 7e51b37 to cb5e3b7 Compare June 8, 2022 09:58
@gsmendoza
Copy link
Copy Markdown
Contributor Author

Closing this for now. I have just noticed that common:test_app already sets up RAILS_ENV to test, but it's not being picked up by Solidus::InstallGenerator. Will investigate.

@gsmendoza gsmendoza closed this Jun 8, 2022
forkata pushed a commit to SuperGoodSoft/solidus_dev_support that referenced this pull request Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Describes or fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant