Bundle less generator failure #1938
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned 
mrcasals
approved these changes
Sep 27, 2017
|
@deivid-rodriguez CircleCI got an error: https://circleci.com/gh/decidim/decidim/4663#queue-placeholder/containers/0
馃槙 |
Codecov Report
@@ Coverage Diff @@
## master #1938 +/- ##
==========================================
- Coverage 98.48% 98.47% -0.02%
==========================================
Files 1145 1145
Lines 25737 25734 -3
==========================================
- Hits 25347 25341 -6
- Misses 390 393 +3 |
mrcasals
suggested changes
Sep 27, 2017
|
@mrcasals It's expected, it's because of d3d76c5. I changed it so that new people cloning a fresh repo are allowed to get an error on DB dropping (because the DB won't exist previously). Previously we workarounded this in CI by skipping this step via What do you think? |
And rename it so that specs don't fail on a freshly cloned repo (where ./tmp does not exist).
We can't asume bundler is available inside our generators, since it will not when they are run through decidim executable. Fixes the following crash: ``` /code/to/decidim/lib/generators/decidim/install_generator.rb:25:in `bundle_install': uninitialized constant Decidim::Generators::InstallGenerator::Bundler (NameError) ```
deivid-rodriguez
force-pushed
the
fix/bundle_less_generator_failure
branch
from
ebd8586
to
08ddb8c
Compare
|
Ah, I understand now. I find it a little weird that we get errors on the build but still everything's normal. Is there a way to check if the database exists before dropping? Found this in a 2-second search on Google: https://stackoverflow.com/a/25592558/2110884 def database_exists?
ActiveRecord::Base.connection
rescue ActiveRecord::NoDatabaseError
false
else
true
endWhat do you think? |
mrcasals
previously approved these changes
Sep 28, 2017
|
I did try that but couldn't get it working. Let me try spend another five minutes! |
For example, on freshly cloned repos. As a result, we no longer need to check for CI here.
So we can give it the standard "test name" and it doesn't affect our "real" development application.
deivid-rodriguez
force-pushed
the
fix/bundle_less_generator_failure
branch
from
08ddb8c
to
a5cf40b
Compare
This was referenced Sep 28, 2017
mrcasals
pushed a commit
that referenced
this pull request
Sep 29, 2017
* Fix crash when running decidim executable We can't asume bundler is available inside our generators, since it will not when they are run through decidim executable. Fixes the following crash: ``` /code/to/decidim/lib/generators/decidim/install_generator.rb:25:in `bundle_install': uninitialized constant Decidim::Generators::InstallGenerator::Bundler (NameError) ``` * Improve some test descriptions * Test generator without flags * Extract a shared example * Extract test app name to a variable And rename it so that specs don't fail on a freshly cloned repo (where ./tmp does not exist). * Allow DB dropping to fail For example, on freshly cloned repos. As a result, we no longer need to check for CI here. * Generate dev application in tests via bin/decidim So we can give it the standard "test name" and it doesn't affect our "real" development application.
Merged
mrcasals
pushed a commit
that referenced
this pull request
Oct 2, 2017
* Fix crash when running decidim executable We can't asume bundler is available inside our generators, since it will not when they are run through decidim executable. Fixes the following crash: ``` /code/to/decidim/lib/generators/decidim/install_generator.rb:25:in `bundle_install': uninitialized constant Decidim::Generators::InstallGenerator::Bundler (NameError) ``` * Improve some test descriptions * Test generator without flags * Extract a shared example * Extract test app name to a variable And rename it so that specs don't fail on a freshly cloned repo (where ./tmp does not exist). * Allow DB dropping to fail For example, on freshly cloned repos. As a result, we no longer need to check for CI here. * Generate dev application in tests via bin/decidim So we can give it the standard "test name" and it doesn't affect our "real" development application.
mrcasals
added a commit
that referenced
this pull request
Oct 5, 2017
* Don't use models in migrations (#1921) * Run seeds in the same process as migrations So we can detect bad model usage inside migrations. * Don't swallow errors in app generation The `rails_command` method shells out the target command, but swallows any errors that may happen. In my opinion, this is bad because we really want these commands to succeed, and since the output is usually long, you might not notice that they have failed. In addition, if errors are swallowed, it's really hard to unit test generators! * Remove hardcoded models from migrations And add a regression test for development app generation. To ensure that these errors can be properly caught in the future. * Prefer `abort` to `exit 1` * Remove unnecessary `ActiveRecord::Base.connection` In migrations, all that stuff in directly available. * Silence test output * Fix app generators (#1931) * Bundle less generator failure (#1938) * Fix crash when running decidim executable We can't asume bundler is available inside our generators, since it will not when they are run through decidim executable. Fixes the following crash: ``` /code/to/decidim/lib/generators/decidim/install_generator.rb:25:in `bundle_install': uninitialized constant Decidim::Generators::InstallGenerator::Bundler (NameError) ``` * Improve some test descriptions * Test generator without flags * Extract a shared example * Extract test app name to a variable And rename it so that specs don't fail on a freshly cloned repo (where ./tmp does not exist). * Allow DB dropping to fail For example, on freshly cloned repos. As a result, we no longer need to check for CI here. * Generate dev application in tests via bin/decidim So we can give it the standard "test name" and it doesn't affect our "real" development application. * Centralize dependencies (#1958) * Style fixes in template Gemfile * Centralize dependencies to a single Gemfile * Upgrade dependencies * Require survey TOS field on admin (#1980) * Require survey TOS field on admin * Add CHANGELOG entry * Fix puma dep * Fix input fields for single locale organizations (#1983) * Fix input fields for single locale organizations * Add CHANGELOG entry * Bump version to 0.6.6 * Downgrade graphiql-rails * Update CHANGELOG * Use latest docker image on CircleCI
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
馃帺 What? Why?
Our generator was crashing when run directly via
decidim's executable (gem install decidim; decidim my_awesome_app). This is becausebundleris not available in that case.This PR fixes that, changes our tests so they catch this thing, and improves some extra related stuff.
馃搶 Related Issues
bundle install聽#1873 (where I introduced this while fixing other issues).馃搵 Subtasks
None.
馃摲 Screenshots (optional)
None.
馃懟 GIF