-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 Gemfile & Dockerfile to get passing tests #1732
Fix Gemfile & Dockerfile to get passing tests #1732
Conversation
@nbulaj this works for me to get clean running tests; the only output besides success is:
|
Changing as follows:
# Raise exceptions instead of rendering exception templates
- config.action_dispatch.show_exceptions = false
+ config.action_dispatch.show_exceptions = :none Fixes that one issue. |
One downside of this is that for every change you make, you need to rebuild the container image, which means reinstalling all the gems, even if they've not changed. |
@ThisIsMissEm I tweaked it back towards the original with no rebuild in 83e6a47 @nbulaj I had to add Gemfile.lock to the docker ignore because my locally git ignored version was causing issues and was out of date compared to the one on the docker image so one test failed. It might be good to add a Gemfile.lock to versioning so that we get predictable gem versions when building / running specs. |
WORKDIR /srv | ||
|
||
COPY Gemfile doorkeeper.gemspec /srv/ | ||
COPY lib/doorkeeper/version.rb /srv/lib/doorkeeper/version.rb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, I tried hacking this whilst working on #1735 and didn't quite get it right, I was doing:
COPY Gemfile Gemfile.lock doorkeeper.gemspec lib/doorkeeper/version.rb /srv/
And it was failing to load the version file; I guess this explains why!
I think the test change and the errors are because you're not doing |
I double-checked and that is not the issue and would include what Gemfile.lock change caused this but I deleted my local one :( |
@ransombriggs if it helps, this is my local gemfile.lock? Gemfile.lock
|
hey 👋 Hm interesting, some new failed test:
Have to 👁️ on that test |
Do we have this commit anywhere in some PR? This error looks noisy now 😞 |
That is confusing when you click on it, but that commit is the "Revert authorization_spec changes" commit in this PR. This test changes behavior with a certain version of a gem, I forget which though, since I deleted my Gemfile.lock when debugging. |
The spec failures are fixed by #1742. Rebase |
I merged in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Summary
This tweaks the Dockerfile and Gemfile to get a passing docker build. This could probably be tweaked back more towards its original style, but it was much easier to debug when I could run the bundle process as the doorkeeper user.