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

Fix issues with test runs #32

Merged
merged 2 commits into from
Dec 16, 2019
Merged

Fix issues with test runs #32

merged 2 commits into from
Dec 16, 2019

Conversation

aldesantis
Copy link
Member

@aldesantis aldesantis commented Dec 13, 2019

This PR contains two fixes which allow us to run the tests for solidus_support both locally and in CI.

ActiveRecord adapters

Fixes the gem for AR adapters not being loaded.

Sprockets

Fixes an issue where Sprockets complains that there's no manifest file by locking to Sprockets 3.

Unfortunately, Sprockets 4, which introduced manifests, doesn't support specifying a custom path for the assets manifest, which obviously doesn't work with in-memory apps.

@kennyadsl submitted a PR which would allow Sprockets to support custom manifest paths, but it was not merged yet.

@aldesantis aldesantis requested a review from a team December 13, 2019 15:57
@aldesantis aldesantis self-assigned this Dec 13, 2019
@kennyadsl
Copy link
Member

@aldesantis can we rebase this one please?

There is an issue with Sprockets 4 not accepting a custom path for
the assets manifest, which doesn't play well with in-memory dummy
apps such as the one we use in this gem.

A fix was provided for sprockets-rails[1] but it was not accepted
yet.

[1]: rails/sprockets-rails#446
@aldesantis aldesantis changed the title Fix Sprockets complaining about the lack of a manifest Fix issues with test runs Dec 14, 2019
@aldesantis
Copy link
Member Author

@kennyadsl done, and also included a fix for AR adapters to allow CircleCI to run with both PostgreSQL and MySQL.

I was a bit torn here, because this gem doesn't really need to run with them and could just use in-memory SQLite, but I figured it's better to have the entire ecosystem run tests with the same stack for consistency. At some point in the future we may introduce DB-dependent changes and we don't want to rely on our memory to remember to change the CI configuration then.

@aldesantis aldesantis merged commit 54cdba6 into solidusio:master Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants