Enhance test experience from external engines#1527
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1527 +/- ##
===========================================
- Coverage 96.94% 73.41% -23.53%
===========================================
Files 496 144 -352
Lines 8404 2355 -6049
===========================================
- Hits 8147 1729 -6418
- Misses 257 626 +369 |
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #1527 +/- ##
===========================================
- Coverage 96.94% 73.41% -23.53%
===========================================
Files 496 144 -352
Lines 8404 2355 -6049
===========================================
- Hits 8147 1729 -6418
- Misses 257 626 +369 |
Codecov Report
@@ Coverage Diff @@
## master #1527 +/- ##
==========================================
+ Coverage 96.94% 96.96% +0.01%
==========================================
Files 496 494 -2
Lines 8404 8391 -13
==========================================
- Hits 8147 8136 -11
+ Misses 257 255 -2 |
|
/cc @amaia |
decidim-dev/lib/decidim/dev.rb
Outdated
|
|
||
| require "letter_opener_web" | ||
| require "decidim/dev/railtie" | ||
| # require "letter_opener_web" |
There was a problem hiding this comment.
Argh.. that was just a test sorry.
There was a problem hiding this comment.
Actually.. I need to remove those lines.
decidim-dev/lib/decidim/dev.rb
Outdated
| require "letter_opener_web" | ||
| require "decidim/dev/railtie" | ||
| # require "letter_opener_web" | ||
| # require "decidim/dev/railtie" |
decidim-dev/lib/decidim/dev.rb
Outdated
|
|
||
| # Public: Get the dummy application path and raises an error if it is not set. | ||
| def self.dummy_app_path | ||
| raise StandardError, "You need to define the `dummy_app_path` first." unless @dummy_app_path |
There was a problem hiding this comment.
Add a more helpful message. SOmething like:
"Please, add Decidim::Dev::dummy_app_path = File.expand_path(File.join("..", "spec", "decidim_dummy_app")) to your spec helper with the path to generate the dummy app"
| require "premailer/rails" | ||
| require "nokogiri" | ||
| require "geocoder" | ||
| require "letter_opener_web" |
There was a problem hiding this comment.
Is this a development only dependency? Why do we need to load it in core?
There was a problem hiding this comment.
Yeah, we should only load this in development. The best way would probably be to load it in decidim-dev's railtie.rb.
There was a problem hiding this comment.
Well.. I'm not sure where I can include this 😄 . It crashes if I try to include it on decidim-dev/lib/dev/dev.rb. It was there in the first place but I needed to move it since I made those changes. Any idea?
There was a problem hiding this comment.
Probably in decidim/dev/railtie.rb
There was a problem hiding this comment.
I'm not sure, but I could try it later.
There was a problem hiding this comment.
To clarify... if I put the require on decidim/lib/decidim/dev.rb (the original file) I cannot run the generate_test_app from an external engine.
There was a problem hiding this comment.
@josepjaume @deivid-rodriguez Fixed! 0a864d3
|
@beagleknight Nice, this is great! And will make @amaia happy! :) |
|
Thanks for taking care of this! 😄 |
|
|
||
| # Public: Get the dummy application path and raises an error if it is not set. | ||
| def self.dummy_app_path | ||
| unless @dummy_app_path |
There was a problem hiding this comment.
@dummy_app_path || raise ?
There was a problem hiding this comment.
It doesn't bork. It raises a syntax error :/
🎩 What? Why?
This adds a few improvements to the test suite:
decidim:generate_test_apptask is available to external engines so you can run it to create the test app in yourspecfolder.dummy_test_apppath was hardcoded so it was impossible to test external engines. Now it must be set in yourspec_helperusing theDecidim::Dev.dummy_app_path=method.Kudos to @josepjaume helping me with this 😄 .
📌 Related Issues
📋 Subtasks
None
📷 Screenshots (optional)
None
👻 GIF