Arvados

Release notes: http://guides.rubyonrails.org/

So far, the most annoying part has been making the websocket tests (or server?) work. Switching to arvados-ws and abandoning the Rails websocket server & tests might be a better use of time.

• serialized attributes behave differently: Rails 4 wants to store [] and {} as NULL.
• attr_protected is replaced by strong parameters

7709-sdk-cli-active_support @ 2fd606b328508babad9af6c0a30c159568b525c2

LGTM, please merge

some extra info: https://gist.github.com/nruth/a3bc1b75281109b036e4

we should do 4.2.5 because of Ubuntu16's default pg is 9.5

we should do 4.2.5 because of Ubuntu16's default pg is 9.5

(Done, and in the meantime we have a workaround for this issue, d31988b6da2d779a8f3ba4ba3c40760f9729c59f)

--

• Rails 4.2.5.2
• Tests pass on Ruby 2.1.10 and 2.3.3
• Jenkins (Ruby 2.1.5p273) https://ci.curoverse.com/job/developer-run-tests/207/

Phew, lots of reading! Some minor comments:

• File services/api/app/controllers/application_controller.rb - Line 491: Was has_key?(...) replaced with key?(...) because it will be deprecated? If so, there’re some other places where it is being used.
• Running tests locally, got this message:
  

  Use assert_nil if expecting nil from services/api/test/functional/arvados/v1/virtual_machines_controller_test.rb:62:in `block in <class:VirtualMachinesControllerTest>'. This will fail in MT6.
  

  
  ... there are some occurrences of assert_equal nil … that could be translated to assert_nil.
• There are other mentions of the previous case, but the warning is not caused by an explicit use of assert_equal nil (not sure if they’re going to cause trouble), for example:
  

  Use assert_nil if expecting nil from services/api/test/unit/log_test.rb:41:in `assert_logged'. This will fail in MT6.
  

• Another deprecation warning running tests:
  

  DEPRECATION WARNING: Passing a nested array to Active Record finder methods is deprecated and will be removed. Flatten your array before using it for 'IN' conditions. (called from readable_job_uuids at /home/lucas/arvados_local/services/api/app/controllers/arvados/v1/nodes_controller.rb:61)
  

Other than that, this LGTM. My local test run passed OK.

Lucas Di Pentima wrote:

• File services/api/app/controllers/application_controller.rb - Line 491: Was has_key?(...) replaced with key?(...) because it will be deprecated? If so, there’re some other places where it is being used.

request.headers is an ActiveSomethingorother::Headers object rather than a plain Hash, and it didn't have has_key?. AFAIK there's no general trend away from has_key?.

Use assert_nil if expecting nil

Ugh, yeah. We have some parameterized tests where the expected value is a variable instead of a literal.

UserTest#test_no_username_set_when_no_base_available = Use assert_nil if expecting nil from /home/tom/src/arvados/services/api/test/unit/user_test.rb:70:in `check_new_username_setting'. This will fail in MT6.
0.03 s = .

  def check_new_username_setting(email_name, expect_name)
    set_user_from_auth :admin
    user = User.create!(email: "#{email_name}@example.org")
    assert_equal(expect_name, user.username)
  end

I've fixed the ones where use a literal nil, and added an assert_equal method that dispatches to assert_nil or super so we're compatible with MT6. 🙄

• Another deprecation warning running tests:
  DEPRECATION WARNING: Passing a nested array to Active Record finder methods is deprecated and will be removed. Flatten your array before using it for 'IN' conditions. (called from readable_job_uuids at /home/lucas/arvados_local/services/api/app/controllers/arvados/v1/nodes_controller.rb:61)

Aha, looks like we were lazy with splat args. Fixed.

7709-api-rails4 @ e1e05845b74ce70712e414830f992ce57d7a8453

This LGTM, thanks!

This happens sporadically

  1) Failure:
Arvados::V1::UsersControllerTest#test_setup_user_with_send_notification_param_false_and_verify_no_email [/home/tom/src/arvados/services/api/test/functional/arvados/v1/users_controller_test.rb:631]:
expected no setup email.
Expected #<Mail::Message:69977645172840, Multipart: false, Headers: <Date: Mon, 10 Apr 2017 13:24:48 -0400>, <From: arvados@example.com>, <To: arvados@example.com>, <Message-ID: <58ebbfe0df821_403f3fa4ecb3311854878@nelle.mail>>, <Subject: Profile created by >, <Mime-Version: 1.0>, <Content-Type: text/plain>, <Content-Transfer-Encoding: 7bit>> to be nil.

Maybe the ActionMailer::Base.deliveries list is not cleared between tests, and if the tests are run in random order, that creates the sporadic failure.
I've found this post that may reflect the same problem: http://stackoverflow.com/questions/21423779/rspec-actionmailerbase-deliveries-should-be-empty

Yes, that fixed it. We were doing that in the User unit tests but never did it in the controller tests.

Replaced the websocket code with a stub that sends a 501 error with a link to the arvados-ws install docs, and then hangs up.

7709-api-rails4 @ de083a9fec0ca08afda5a9369c6cd32dbdcd0965

Sorry for the delay, lgtm!