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.

Sign up for GitHub

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

Jump to bottom

Rails 4.1 upgrade #1799

Merged
merged 59 commits into from
Dec 7, 2017
Merged

Rails 4.1 upgrade #1799

merged 59 commits into from
Dec 7, 2017

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Nov 26, 2017 •
edited
Loading

@ryzokuken @icarito -- trying to tackle this here. Any PRs against this one welcome!

These are changes that are not compatible with Rails 3, unlike #1798.

Heavily based on #1160 by @sjones88

@jywarren
Copy link
Member Author

Also can't seem to add /bin/*... yet https://stackoverflow.com/questions/18591399/should-i-add-the-rails-4-bin-directory-to-git#18591998

@jywarren jywarren mentioned this pull request Nov 26, 2017
Closed
@publiclab publiclab deleted a comment from PublicLabBot Nov 26, 2017
@PublicLabBot
Copy link

PublicLabBot commented Nov 26, 2017 •
edited
Loading

2 Messages
📖 @jywarren Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the body of your PR, in the format: Fixes #123.

Generated by 🚫 Danger

@jywarren
Copy link
Member Author

Most of the errors are either:

ArgumentError: invalid date (as in, for example, WikiControllerTest#test_admin_user_should_delete_wiki_page:)

or:

WikiControllerTest#test_admin_user_should_delete_wiki_page:
Authlogic::Session::Activation::NotActivatedError: You must activate the Authlogic::Session::Base.controller with a controller object before creating objects
    test/functional/wiki_controller_test.rb:12:in `teardown'

@jywarren
Copy link
Member Author

also noting that Dangerbot is not seeing errors correctly because the format has changed.

@jywarren
Copy link
Member Author

OK, the fixtures.rb code in ActiveRecord is trying to load ruser.rb instead of user.rb based on, apparently, a scan of the fixtures folder.

@jywarren
Copy link
Member Author

OK, moving on to ones like:

ERROR["test_should_get_show_for_root_page", TalkControllerTest, 2.7286774160020286]
 test_should_get_show_for_root_page#TalkControllerTest (2.73s)
ActionView::Template::Error:         ActionView::Template::Error: SQLite3::SQLException: no such column: community_tags.nid: SELECT "term_data".* FROM "term_data"  WHERE (community_tags.nid = 2 AND name LIKE 'style:fancy')
            app/models/node.rb:382:in `has_tag'
            app/views/layouts/application.html.erb:25:in `_app_views_layouts_application_html_erb___1017683525_53195436'
            test/functional/talk_controller_test.rb:6:in `block in <class:TalkControllerTest>'

ERROR["test_should_assign_correct_value_to_graph_comments_on_GET_stats", StatsControllerTest, 4.533888999008923]
 test_should_assign_correct_value_to_graph_comments_on_GET_stats#StatsControllerTest (4.53s)
ActiveRecord::StatementInvalid:         ActiveRecord::StatementInvalid: SQLite3::SQLException: wrong number of arguments to function COUNT(): SELECT COUNT(created, type, status) FROM "node"  WHERE "node"."type" = 'note' AND "node"."status" = 1 AND ("node"."created" BETWEEN 1511142110 AND 1511746910)
            app/controllers/stats_controller.rb:42:in `index'
            test/functional/stats_controller_test.rb:7:in `block in <class:StatsControllerTest>'

@jywarren
Copy link
Member Author

311 tests, 411 assertions, 6 failures, 179 errors, 0 skips

@jywarren
Copy link
Member Author

Better now: 311 tests, 652 assertions, 6 failures, 100 errors, 0 skips

@jywarren
Copy link
Member Author

Note this new process for secrets: http://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#config-secrets-yml

@jywarren jywarren mentioned this pull request Nov 27, 2017
Closed
8 tasks
@jywarren
Copy link
Member Author

I'm pushing this to unstable for testing, as the last remaining issues are with Solr!

@jywarren
Copy link
Member Author

@icarito, looks like we have to do a different setup for our secrets, how does this work in production mode containers like unstable?

Missing secret_token and secret_key_base for 'production' environment, set these values in config/secrets.yml

@jywarren
Copy link
Member Author

Oh, I tried running RAILS_ENV=production SOLR_URL=http://solr:8983/solr/default/ docker-compose -f docker-compose-unstable.yml exec web rake test:solr on unstable and this triggered a reindexing, which I canceled (it would've taken too long).

Hmm, not sure how to debug these solr errors:

[##################################] [92/92] [100.00%] [00:01] [00:00] [53.27/s]/usr/local/bin/ruby -I"lib:test" -I"/usr/local/bundle/gems/rake-10.5.0/lib" "/usr/local/bundle/gems/rake-10.5.0/lib/rake/rake_test_loader.rb" "test/solr/*_test.rb"  /usr/local/bundle/gems/ci_reporter_test_unit-1.0.1/lib/ci/reporter/rake/test_unit_loader.rb
[Coveralls] Using SimpleCov's default settings.
/usr/local/bundle/gems/activerecord-4.1.16/lib/active_record/associations/builder/collection_association.rb:38: warning: already initialized constant TagNodeAssociationExtension
/usr/local/bundle/gems/activerecord-4.1.16/lib/active_record/associations/builder/collection_association.rb:38: warning: previous definition of TagNodeAssociationExtension was here
Started with run options --seed 60657
 FAIL["test_Node.search_for_two_different_key_words_returns_different_results", NodeSearchTest, 0.20476326099998232]
 test_Node.search_for_two_different_key_words_returns_different_results#NodeSearchTest (0.20s)
        Expected false to be truthy.
        test/solr/node_search_test.rb:30:in `block in <class:NodeSearchTest>'
 FAIL["test_should_get_search_test_action", SearchesControllerTest, 2.7980125459999954]
 test_should_get_search_test_action#SearchesControllerTest (2.80s)
        Expected [] to not be equal to [].
        test/solr/searches_controller_test.rb:23:in `block in <class:SearchesControllerTest>'
 FAIL["test_running_TypeaheadService.notes", TypeaheadServiceTest, 2.857186628999955]
 test_running_TypeaheadService.notes#TypeaheadServiceTest (2.86s)
        Expected: 1
          Actual: 2
        test/solr/typeahead_service_test.rb:20:in `block in <class:TypeaheadServiceTest>'
 FAIL["test_running_TypeaheadService.search_all", TypeaheadServiceTest, 2.9072877629999994]
 test_running_TypeaheadService.search_all#TypeaheadServiceTest (2.91s)
        Expected: 0
          Actual: 1
        test/solr/typeahead_service_test.rb:13:in `block in <class:TypeaheadServiceTest>'
  7/7: [===================================] 100% Time: 00:00:02, Time: 00:00:02

@jywarren
Copy link
Member Author

Whoa, but perhaps Rails 4 is running right now on unstable? Can you confirm @icarito? I checked and the gemfile says yes...

@jywarren
Copy link
Member Author

I manually set the secrets.yml just after the build started.

@jywarren
Copy link
Member Author

And this doesn't load, so maybe Solr is not running? http://unstable.publiclab.org/searches/test

@jywarren
Copy link
Member Author

Nope, unrelated -- AbstractController::ActionNotFound (The action 'show' could not be found for SearchesController): -- must be a routes issue related to the new code.

@jywarren
Copy link
Member Author

http://unstable.publiclab.org/api/typeahead/all?srchString=precocious doesn't return anything but this could just be due to the partial reindex...

@jywarren
Copy link
Member Author

Manually testing Node.search { fulltext 'blog' } for example, on unstable, returns results.

@jywarren
Copy link
Member Author

I'm trying now to use this to load test fixtures into the test db and open a rails console in test mode:

rake db:fixtures:load RAILS_ENV=test && RAILS_ENV=test bundle exec rails c

At the end of:

RAILS_ENV=production SOLR_URL=http://solr:8983/solr/default/ docker-compose -f docker-compose-unstable.yml exec web rake db:fixtures:load RAILS_ENV=test && RAILS_ENV=test bundle exec rails c

Not sure this'll work!

@jywarren
Copy link
Member Author

... it didn't, it just hung. This ran, but in production mode: RAILS_ENV=test SOLR_URL=http://solr:8983/solr/default/ docker-compose -f docker-compose-unstable.yml exec web rails c

@jywarren
Copy link
Member Author

Hmm, honestly @icarito @ryzokuken i'm a bit stuck on this... why wouldn't the solr tests be passing, esp. if I can run the queries in production mode on the console? Any fresh ideas very welcome! We're VERY close on this... all normal tests are passing, for Rails 4.1

@jywarren
Copy link
Member Author

Can we look at the solr logs for the test run to see what queries are actually being triggered by the solr tests?

@icarito
Copy link
Member

icarito commented Nov 28, 2017 via email

@jywarren
Copy link
Member Author

jywarren commented Nov 28, 2017 via email

@jywarren
Copy link
Member Author

I added it to .travis.yml -- let's see!

@icarito
Copy link
Member

icarito commented Nov 29, 2017

It doesn't appear to have run the docker-compose logs command?

@icarito
Copy link
Member

icarito commented Nov 29, 2017

Oh I see it... line 3206, reading...

@jywarren
Copy link
Member Author

jywarren commented Dec 6, 2017

Ummmm, i think we did it!!! There's only this last error due to the Dangerfile and Dangerbot not being able to parse the error output!

@ryzokuken
Copy link
Member

ryzokuken commented Dec 6, 2017 via email

@jywarren
Copy link
Member Author

jywarren commented Dec 6, 2017

Adding a rescue around that part of the Dangerfile, with console logging error. But i want to address that separately.

@jywarren
Copy link
Member Author

jywarren commented Dec 6, 2017

The command "tail -n +2 -q ./test/reports/TEST*.xml >> output.xml" exited with 1.

This is based on https://github.com/jywarren/plots2/blob/rails-4/.travis.yml#L22

  - echo -e '<?xml version="1.0" encoding="UTF-8"?>' > output.xml

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017

YAYYYYYYYYY @icarito i'm merging this in now and will push to staging.

@jywarren jywarren merged commit 5ffd7c6 into publiclab:master Dec 7, 2017
@ryzokuken
Copy link
Member

ryzokuken commented Dec 7, 2017 via email

@jywarren jywarren mentioned this pull request Dec 7, 2017
Merged
2 tasks
@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017

I just pushed it there. Fingers X and if you can report back here if it works for you, thank you in advance!

it'll show up at http://staging.publiclab.org/

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017

Hmm, @icarito i see Incomplete response received from application -- any idea why?

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017

Ah! we need to set SECRET_KEY_BASE -- pretty straightforward.

Your rails_env production don't have required set up,probably missing secret_key_base.

https://blog.carbonfive.com/2015/03/17/docker-rails-docker-compose-together-in-your-development-workflow/ has guidance. Is this something you could work on?

Thank you!

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017

http://guides.rubyonrails.org/upgrading_ruby_on_rails.html#config-secrets-yml

@icarito
Copy link
Member

icarito commented Dec 7, 2017 via email

@ryzokuken
Copy link
Member

ryzokuken commented Dec 7, 2017 via email

@icarito
Copy link
Member

icarito commented Dec 7, 2017

Okay I setup the environment variable and we're good!
It is still quite slow for me, although the server load is fine.

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017 via email

@icarito
Copy link
Member

icarito commented Dec 7, 2017 via email

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2017 via email

@jywarren
Copy link
Member Author

jywarren commented Dec 8, 2017 •
edited
Loading

This is pretty weird, but some routes got lost in the upgrade (bad job merging by me) but the tests didn't catch it! Hmm... including tag#gridsEmbed and one subscriptions in these commits:

However, the tests show:

573 tests, 1699 assertions, 0 failures, 0 errors, 0 skips (here)

Vs. the last version before this PR:

183 tests, 546 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
311 tests, 800 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
74 tests, 214 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications

That equals

568 tests, 1560 assertions

(here)

@jywarren
Copy link
Member Author

jywarren commented Dec 8, 2017

Update -- i think functional tests do not test routes: #1841

@grvsachdeva grvsachdeva mentioned this pull request Jan 10, 2019
Merged
5 tasks
@jywarren jywarren mentioned this pull request Feb 26, 2019
Open
20 tasks
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
@jywarren
Rails 4.1 upgrade (publiclab#1799)
4cc4ac1 
* rails 4 find_by and assert_not conventions

* assert_not prep for rails 4

* bin, form_for, and other rails 4 reqs

* config changes for rails 4

* regex no longer multiline

* non-multiline regex pt 2

* further regex fixes

* missing parenthesis

* last regex fix, i hope

* added bin/

* resolved plurality issues

* fixtures disentangling

* passing more functional tests

* further fixtures and table name fixes

* major refactor of .references() after actually reading docs

* further debugging

* down to 86 functional test errors

* fixed date helpers

* down to 36 functional test errors

* down to 50 functional test errors

* down to 6 errors

* down to 1 functional test errors

* addressing functional test errors

* getting closer

* user sessions fixes

* integration and unit test error resolutions

* down to 2 failures overall

* all tests passing!

* small fix

* searches test route

* small changes

* adding travis solr log output

* Update routes.rb

* Update .travis.yml

* Update searches_controller_test.rb

* Update .travis.yml

* Update searches_controller_test.rb

* Update node_search_test.rb

* Update node_search_test.rb

* Update searches_controller_test.rb

* moved custom rake tasks to Rakefile

* Update node_search_test.rb

* reactivate default test tasks

* removed inline reindex

* rake test:run tweak

* invoke instead of execute rake tasks

* sorted rakefile issues maybe

* test adjustments

* better rakefile output

* execute instead of invoke in Rakefile

* reworking namespace

* Update Rakefile

* Update Rakefile

* Update Dangerfile

* Create TEST_.xml

* Delete TEST_.xml

* Update .travis.yml

* Update .travis.yml

* Update .travis.yml
@jywarren jywarren mentioned this pull request May 10, 2019
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jywarren @PublicLabBot @icarito @ryzokuken