Add tests for controllers #615
Conversation
|
Code Climate has analyzed commit c5c5524 and detected 40 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Codecov Report
@@ Coverage Diff @@
## main #615 +/- ##
=====================================
Coverage 41.1% 41.1%
=====================================
Files 34 34
Lines 1321 1321
=====================================
Hits 543 543
Misses 778 778
|
|
The |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@publiclab/reviewers This is ready for merge. |
| @@ -40,7 +40,7 @@ def self.histogram_cm_per_pixel | |||
|
|
|||
| def self.histogram_cm_per_pixel_in_tens | |||
| e = Export.where('cm_per_pixel != "" AND cm_per_pixel < 500') | |||
| .order(cm_per_pixel: 'desc') | |||
| .order('cm_per_pixel desc') | |||
There was a problem hiding this comment.
I would prefer cm_per_pixel: :desc because it is rails way.
Would love to hear from you 😄 .
There was a problem hiding this comment.
@jainaman224 https://travis-ci.org/publiclab/mapknitter/builds/539536753#L1246-L1249
The test fails when I do that. Not sure why :/
There was a problem hiding this comment.
I think this is related to our MySQL adapter gem version.
|
Hi! Sorry I missed this! Can you resolve conflicts, and could we get a review from someone else in @publiclab/mapknitter-reviewers as well? Thanks so much, this is great! Regarding sessions, perhaps we could ask @SidharthBansal to offer input on how we might test those? |
|
Is this ready to merge? @kaustubh-nair |
Yes it is. Can you review it if you have time? |
.rubocop.yml
Outdated
| @@ -0,0 +1,10 @@ | |||
| Metrics/BlockLength: | |||
There was a problem hiding this comment.
Maybe this is not needed since #547 is ready to merge too. And we're using our standard stylesheet.
| # assert_includes @response.body, @annotation.text | ||
| # end | ||
|
|
||
| test 'should update annotations' do |
There was a problem hiding this comment.
The update and destroy actions don't work
So i left them blank for now.
| test 'should update annotations' do | ||
| end | ||
|
|
||
| test 'should destroy annotations' do |
| def setup | ||
| @map = maps(:saugus) | ||
| end | ||
|
|
||
| def teardown | ||
| end | ||
|
|
||
| test "index" do | ||
| test 'index' do |
There was a problem hiding this comment.
Maybe a more significant and standardized test names would be bettter, such as test 'should visit Export index page'
| end | ||
|
|
||
| test "cancel fails if not logged in" do | ||
| test 'cancel fails if not logged in' do |
There was a problem hiding this comment.
| assert assigns[:map] | ||
| assert_equal 'text/html', @response.content_type | ||
| assert flash.empty? | ||
| end | ||
|
|
||
| test "cancels export" do | ||
| test 'cancels export' do |
There was a problem hiding this comment.
| session[:user_id] = 1 | ||
| get :cancel, id: @map.id | ||
| assert_response :success | ||
| assert_equal 'cancelled', @response.body | ||
| assert assigns[:map] | ||
| end | ||
|
|
||
| test "exports cancelled if present" do | ||
| test 'exports cancelled if present' do |
There was a problem hiding this comment.
| session[:user_id] = 1 | ||
| get :cancel, id: @map.id, exports: 'cess' | ||
| assert_response :redirect | ||
| assert flash.present? | ||
| assert_redirected_to '/exports' | ||
| end | ||
|
|
||
| test "progress" do | ||
| test 'progress' do |
There was a problem hiding this comment.
| get :progress, id: @map.id | ||
| assert_response :success | ||
| assert_equal 'export not running', @response.body | ||
| assert_equal 'text/html', @response.content_type | ||
| end | ||
|
|
||
| test 'progress with no export' do |
There was a problem hiding this comment.
| assert_equal 'export has not been run', @response.body | ||
| end | ||
|
|
||
| test 'progress completed' do |
There was a problem hiding this comment.
| assert_equal 'complete', @response.body | ||
| end | ||
|
|
||
| test 'progress failed' do |
There was a problem hiding this comment.
| @@ -25,100 +24,100 @@ def teardown | |||
| assert_equal image_urls, json_response[0]['image_urls'] | |||
| end | |||
|
|
|||
| test "should get index" do | |||
| test 'should get index' do | |||
There was a problem hiding this comment.
| require 'test_helper' | ||
|
|
||
| class UtilityControllerTest < ActionController::TestCase | ||
| test 'tms_alt' do |
There was a problem hiding this comment.
Just did that @kaustubh-nair. Check out my comments, despite of those test names I think everything is ok. |
|
OK! Just ping me (preferably in Gitter actually as I'm working offline a lot today) and we can merge this! |
|
Okay! |
1402a9e to
09d64d9
Compare
09d64d9 to
6975a77
Compare
|
All ready for a merge here now? Just checking since I saw a few more commits come in! |
|
Feel free to label it |
|
Yes @jywarren this is ready 👍 |
|
Super!!! |
* add utility test * Add exports and annotations controller tests' * Add tests for images * Fix codeclimate issues * Single quoted strings, huh :/ * Fix export controller test * Fix histogram ordering * Fix codeclimate issues * Fix image controller tests * Small fixes * Add rubocop.yml * Fix rubocop * Increase linelength * Fix rubocop * Rename tests for better readability * Rename tests * Fixing Codeclimate offenses
Part of #594