[ARVADOS] updated: f3460b2f2e8088e861d1852e4f26784b3c96ded8

git at public.curoverse.com git at public.curoverse.com
Wed Nov 26 23:27:44 EST 2014


Summary of changes:
 .../actions_controller_test.rb                     |   0
 .../api_client_authorizations_controller_test.rb   |   0
 .../application_controller_test.rb                 |   4 +
 .../authorized_keys_controller_test.rb             |   0
 .../controllers/collections_controller_test.rb     | 231 +++++++++++++++++++++
 .../groups_controller_test.rb                      |   0
 .../humans_controller_test.rb                      |   0
 .../job_tasks_controller_test.rb                   |   0
 .../jobs_controller_test.rb                        |   0
 .../keep_disks_controller_test.rb                  |   0
 .../links_controller_test.rb                       |   0
 .../logs_controller_test.rb                        |   0
 .../nodes_controller_test.rb                       |   0
 .../pipeline_instances_controller_test.rb          | 171 ++++++++++++---
 .../pipeline_templates_controller_test.rb          |   0
 .../projects_controller_test.rb                    |   0
 .../repositories_controller_test.rb                |   0
 .../sessions_controller_test.rb                    |   0
 .../specimens_controller_test.rb                   |   0
 .../traits_controller_test.rb                      |   0
 .../user_agreements_controller_test.rb             |   0
 .../users_controller_test.rb                       |   2 +-
 .../virtual_machines_controller_test.rb            |   0
 apps/workbench/test/diagnostics_test_helper.rb     |   4 +
 apps/workbench/test/functional/.gitkeep            |   0
 .../test/functional/collections_controller_test.rb | 229 --------------------
 .../pipeline_instances_controller_test.rb          | 157 --------------
 .../test/helpers/pipeline_instances_helper_test.rb |  38 ++++
 .../test/integration/application_layout_test.rb    |   5 +
 .../workbench/test/integration/collections_test.rb |   2 -
 .../test/integration/pipeline_instances_test.rb    |   2 -
 apps/workbench/test/integration/projects_test.rb   |   8 +-
 .../test/integration/user_agreements_test.rb       |   2 -
 .../test/integration/user_manage_account_test.rb   |   2 -
 .../test/integration/user_profile_test.rb          |   2 -
 apps/workbench/test/integration/users_test.rb      |   1 -
 apps/workbench/test/test_helper.rb                 |  53 ++++-
 doc/sdk/cli/subcommands.html.textile.liquid        | 127 ++++++-----
 sdk/cli/Gemfile.lock                               |  40 ++--
 .../api/app/controllers/application_controller.rb  |  35 ++++
 .../arvados/v1/keep_disks_controller.rb            |  14 +-
 .../app/controllers/arvados/v1/nodes_controller.rb |   2 +-
 .../controllers/arvados/v1/schema_controller.rb    |   8 +-
 services/api/db/structure.sql                      |   2 +-
 .../test/functional/application_controller_test.rb |  44 ++++
 .../arvados/v1/job_reuse_controller_test.rb        |  38 ++--
 .../arvados/v1/keep_disks_controller_test.rb       |  31 ++-
 services/api/test/integration/remote_reset_test.rb |  41 ++--
 48 files changed, 718 insertions(+), 577 deletions(-)
 rename apps/workbench/test/{functional => controllers}/actions_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/api_client_authorizations_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/application_controller_test.rb (98%)
 rename apps/workbench/test/{functional => controllers}/authorized_keys_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/groups_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/humans_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/job_tasks_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/jobs_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/keep_disks_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/links_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/logs_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/nodes_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/pipeline_templates_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/projects_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/repositories_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/sessions_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/specimens_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/traits_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/user_agreements_controller_test.rb (100%)
 rename apps/workbench/test/{functional => controllers}/users_controller_test.rb (96%)
 rename apps/workbench/test/{functional => controllers}/virtual_machines_controller_test.rb (100%)
 delete mode 100644 apps/workbench/test/functional/.gitkeep
 delete mode 100644 apps/workbench/test/functional/collections_controller_test.rb
 delete mode 100644 apps/workbench/test/functional/pipeline_instances_controller_test.rb
 create mode 100644 apps/workbench/test/helpers/pipeline_instances_helper_test.rb

       via  f3460b2f2e8088e861d1852e4f26784b3c96ded8 (commit)
       via  7e828d4672d2b306bccacfed309123c81846d565 (commit)
       via  437d4225617df22b4e6d9c245f854620cdc09bc7 (commit)
       via  e3914c4981a48d086562bcdc45f0d385f95fc083 (commit)
       via  a69a1cb084e6feb9e4e9c6538a52289e7c56700b (commit)
       via  76eaaac9fed4e74b8073ee08f99b84fc7922ac9f (commit)
       via  84efd065eba386e98e07cdd232dc172e169af451 (commit)
       via  7b8db198ad4cf91e605f099f78f5c4a1bef152ff (commit)
       via  257ecfece0f6941011c85e735459d86b9f850d25 (commit)
       via  333402104e6b7a163bf3f8483a928dbe571b5c2c (commit)
       via  60cf64002cee6af43fe8b6a122c104a12c1fd7bf (commit)
       via  1a4282c6d30b209a882c255e0d5777851ff6f034 (commit)
       via  f3d43ab311114a7c25b7ecc47f63affdc7197efb (commit)
       via  a8c9797de0fac6cc28d04daeade83d5e0c558076 (commit)
       via  ac4cdfc2577b9d25ccbc9ac5d8f0333a81102367 (commit)
      from  e65d698ab39cb44cf12630498ac75b475bedec44 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit f3460b2f2e8088e861d1852e4f26784b3c96ded8
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Nov 26 20:02:36 2014 -0500

    4533: Move all controller tests into controllers/, merge overlapping class defs.

diff --git a/apps/workbench/test/functional/actions_controller_test.rb b/apps/workbench/test/controllers/actions_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/actions_controller_test.rb
rename to apps/workbench/test/controllers/actions_controller_test.rb
diff --git a/apps/workbench/test/functional/api_client_authorizations_controller_test.rb b/apps/workbench/test/controllers/api_client_authorizations_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/api_client_authorizations_controller_test.rb
rename to apps/workbench/test/controllers/api_client_authorizations_controller_test.rb
diff --git a/apps/workbench/test/functional/application_controller_test.rb b/apps/workbench/test/controllers/application_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/application_controller_test.rb
rename to apps/workbench/test/controllers/application_controller_test.rb
diff --git a/apps/workbench/test/functional/authorized_keys_controller_test.rb b/apps/workbench/test/controllers/authorized_keys_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/authorized_keys_controller_test.rb
rename to apps/workbench/test/controllers/authorized_keys_controller_test.rb
diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb
index a5c6033..9f4c5da 100644
--- a/apps/workbench/test/controllers/collections_controller_test.rb
+++ b/apps/workbench/test/controllers/collections_controller_test.rb
@@ -1,8 +1,239 @@
 require 'test_helper'
 
 class CollectionsControllerTest < ActionController::TestCase
+  # These tests don't do state-changing API calls. Save some time by
+  # skipping the database reset.
+  reset_api_fixtures :after_each_test, false
+  reset_api_fixtures :after_suite, true
+
   include PipelineInstancesHelper
 
+  NONEXISTENT_COLLECTION = "ffffffffffffffffffffffffffffffff+0"
+
+  def stub_file_content
+    # For the duration of the current test case, stub file download
+    # content with a randomized (but recognizable) string. Return the
+    # string, the test case can use it in assertions.
+    txt = 'the quick brown fox ' + rand(2**32).to_s
+    @controller.stubs(:file_enumerator).returns([txt])
+    txt
+  end
+
+  def collection_params(collection_name, file_name=nil)
+    uuid = api_fixture('collections')[collection_name.to_s]['uuid']
+    params = {uuid: uuid, id: uuid}
+    params[:file] = file_name if file_name
+    params
+  end
+
+  def assert_hash_includes(actual_hash, expected_hash, msg=nil)
+    expected_hash.each do |key, value|
+      assert_equal(value, actual_hash[key], msg)
+    end
+  end
+
+  def assert_no_session
+    assert_hash_includes(session, {arvados_api_token: nil},
+                         "session includes unexpected API token")
+  end
+
+  def assert_session_for_auth(client_auth)
+    api_token =
+      api_fixture('api_client_authorizations')[client_auth.to_s]['api_token']
+    assert_hash_includes(session, {arvados_api_token: api_token},
+                         "session token does not belong to #{client_auth}")
+  end
+
+  def show_collection(params, session={}, response=:success)
+    params = collection_params(params) if not params.is_a? Hash
+    session = session_for(session) if not session.is_a? Hash
+    get(:show, params, session)
+    assert_response response
+  end
+
+  test "viewing a collection" do
+    show_collection(:foo_file, :active)
+    assert_equal([['.', 'foo', 3]], assigns(:object).files)
+  end
+
+  test "viewing a collection fetches related projects" do
+    show_collection({id: api_fixture('collections')["foo_file"]['portable_data_hash']}, :active)
+    assert_includes(assigns(:same_pdh).map(&:owner_uuid),
+                    api_fixture('groups')['aproject']['uuid'],
+                    "controller did not find linked project")
+  end
+
+  test "viewing a collection fetches related permissions" do
+    show_collection(:bar_file, :active)
+    assert_includes(assigns(:permissions).map(&:uuid),
+                    api_fixture('links')['bar_file_readable_by_active']['uuid'],
+                    "controller did not find permission link")
+  end
+
+  test "viewing a collection fetches jobs that output it" do
+    show_collection(:bar_file, :active)
+    assert_includes(assigns(:output_of).map(&:uuid),
+                    api_fixture('jobs')['foobar']['uuid'],
+                    "controller did not find output job")
+  end
+
+  test "viewing a collection fetches jobs that logged it" do
+    show_collection(:baz_file, :active)
+    assert_includes(assigns(:log_of).map(&:uuid),
+                    api_fixture('jobs')['foobar']['uuid'],
+                    "controller did not find logger job")
+  end
+
+  test "viewing a collection fetches logs about it" do
+    show_collection(:foo_file, :active)
+    assert_includes(assigns(:logs).map(&:uuid),
+                    api_fixture('logs')['log4']['uuid'],
+                    "controller did not find related log")
+  end
+
+  test "viewing collection files with a reader token" do
+    params = collection_params(:foo_file)
+    params[:reader_token] = api_fixture("api_client_authorizations",
+                                        "active_all_collections", "api_token")
+    get(:show_file_links, params)
+    assert_response :success
+    assert_equal([['.', 'foo', 3]], assigns(:object).files)
+    assert_no_session
+  end
+
+  test "fetching collection file with reader token" do
+    expected = stub_file_content
+    params = collection_params(:foo_file, "foo")
+    params[:reader_token] = api_fixture("api_client_authorizations",
+                                        "active_all_collections", "api_token")
+    get(:show_file, params)
+    assert_response :success
+    assert_equal(expected, @response.body,
+                 "failed to fetch a Collection file with a reader token")
+    assert_no_session
+  end
+
+  test "reader token Collection links end with trailing slash" do
+    # Testing the fix for #2937.
+    session = session_for(:active_trustedclient)
+    post(:share, collection_params(:foo_file), session)
+    assert(@controller.download_link.ends_with? '/',
+           "Collection share link does not end with slash for wget")
+  end
+
+  test "getting a file from Keep" do
+    params = collection_params(:foo_file, 'foo')
+    sess = session_for(:active)
+    expect_content = stub_file_content
+    get(:show_file, params, sess)
+    assert_response :success
+    assert_equal(expect_content, @response.body,
+                 "failed to get a correct file from Keep")
+  end
+
+  test "can't get a file from Keep without permission" do
+    params = collection_params(:foo_file, 'foo')
+    sess = session_for(:spectator)
+    get(:show_file, params, sess)
+    assert_response 404
+  end
+
+  test "trying to get a nonexistent file from Keep returns a 404" do
+    params = collection_params(:foo_file, 'gone')
+    sess = session_for(:admin)
+    get(:show_file, params, sess)
+    assert_response 404
+  end
+
+  test "getting a file from Keep with a good reader token" do
+    params = collection_params(:foo_file, 'foo')
+    read_token = api_fixture('api_client_authorizations')['active']['api_token']
+    params[:reader_token] = read_token
+    expect_content = stub_file_content
+    get(:show_file, params)
+    assert_response :success
+    assert_equal(expect_content, @response.body,
+                 "failed to get a correct file from Keep using a reader token")
+    assert_not_equal(read_token, session[:arvados_api_token],
+                     "using a reader token set the session's API token")
+  end
+
+  test "trying to get from Keep with an unscoped reader token prompts login" do
+    params = collection_params(:foo_file, 'foo')
+    params[:reader_token] =
+      api_fixture('api_client_authorizations')['active_noscope']['api_token']
+    get(:show_file, params)
+    assert_response :redirect
+  end
+
+  test "can get a file with an unpermissioned auth but in-scope reader token" do
+    params = collection_params(:foo_file, 'foo')
+    sess = session_for(:expired)
+    read_token = api_fixture('api_client_authorizations')['active']['api_token']
+    params[:reader_token] = read_token
+    expect_content = stub_file_content
+    get(:show_file, params, sess)
+    assert_response :success
+    assert_equal(expect_content, @response.body,
+                 "failed to get a correct file from Keep using a reader token")
+    assert_not_equal(read_token, session[:arvados_api_token],
+                     "using a reader token set the session's API token")
+  end
+
+  test "inactive user can retrieve user agreement" do
+    ua_collection = api_fixture('collections')['user_agreement']
+    # Here we don't test whether the agreement can be retrieved from
+    # Keep. We only test that show_file decides to send file content,
+    # so we use the file content stub.
+    stub_file_content
+    get :show_file, {
+      uuid: ua_collection['uuid'],
+      file: ua_collection['manifest_text'].match(/ \d+:\d+:(\S+)/)[1]
+    }, session_for(:inactive)
+    assert_nil(assigns(:unsigned_user_agreements),
+               "Did not skip check_user_agreements filter " +
+               "when showing the user agreement.")
+    assert_response :success
+  end
+
+  test "requesting nonexistent Collection returns 404" do
+    show_collection({uuid: NONEXISTENT_COLLECTION, id: NONEXISTENT_COLLECTION},
+                    :active, 404)
+  end
+
+  test "use a reasonable read buffer even if client requests a huge range" do
+    fakefiledata = mock
+    IO.expects(:popen).returns(fakefiledata)
+    fakefiledata.expects(:read).twice.with() do |length|
+      # Fail the test if read() is called with length>1MiB:
+      length < 2**20
+      ## Force the ActionController::Live thread to lose the race to
+      ## verify that @response.body.length actually waits for the
+      ## response (see below):
+      # sleep 3
+    end.returns("foo\n", nil)
+    fakefiledata.expects(:close)
+    foo_file = api_fixture('collections')['foo_file']
+    @request.headers['Range'] = 'bytes=0-4294967296/*'
+    get :show_file, {
+      uuid: foo_file['uuid'],
+      file: foo_file['manifest_text'].match(/ \d+:\d+:(\S+)/)[1]
+    }, session_for(:active)
+    # Wait for the whole response to arrive before deciding whether
+    # mocks' expectations were met. Otherwise, Mocha will fail the
+    # test depending on how slowly the ActionController::Live thread
+    # runs.
+    @response.body.length
+  end
+
+  test "show file in a subdirectory of a collection" do
+    params = collection_params(:collection_with_files_in_subdir, 'subdir2/subdir3/subdir4/file1_in_subdir4.txt')
+    expect_content = stub_file_content
+    get(:show_file, params, session_for(:user1_with_load))
+    assert_response :success
+    assert_equal(expect_content, @response.body, "failed to get a correct file from Keep")
+  end
+
   test 'provenance graph' do
     use_token 'admin'
 
diff --git a/apps/workbench/test/functional/groups_controller_test.rb b/apps/workbench/test/controllers/groups_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/groups_controller_test.rb
rename to apps/workbench/test/controllers/groups_controller_test.rb
diff --git a/apps/workbench/test/functional/humans_controller_test.rb b/apps/workbench/test/controllers/humans_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/humans_controller_test.rb
rename to apps/workbench/test/controllers/humans_controller_test.rb
diff --git a/apps/workbench/test/functional/job_tasks_controller_test.rb b/apps/workbench/test/controllers/job_tasks_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/job_tasks_controller_test.rb
rename to apps/workbench/test/controllers/job_tasks_controller_test.rb
diff --git a/apps/workbench/test/functional/jobs_controller_test.rb b/apps/workbench/test/controllers/jobs_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/jobs_controller_test.rb
rename to apps/workbench/test/controllers/jobs_controller_test.rb
diff --git a/apps/workbench/test/functional/keep_disks_controller_test.rb b/apps/workbench/test/controllers/keep_disks_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/keep_disks_controller_test.rb
rename to apps/workbench/test/controllers/keep_disks_controller_test.rb
diff --git a/apps/workbench/test/functional/links_controller_test.rb b/apps/workbench/test/controllers/links_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/links_controller_test.rb
rename to apps/workbench/test/controllers/links_controller_test.rb
diff --git a/apps/workbench/test/functional/logs_controller_test.rb b/apps/workbench/test/controllers/logs_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/logs_controller_test.rb
rename to apps/workbench/test/controllers/logs_controller_test.rb
diff --git a/apps/workbench/test/functional/nodes_controller_test.rb b/apps/workbench/test/controllers/nodes_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/nodes_controller_test.rb
rename to apps/workbench/test/controllers/nodes_controller_test.rb
diff --git a/apps/workbench/test/controllers/pipeline_instances_controller_test.rb b/apps/workbench/test/controllers/pipeline_instances_controller_test.rb
index b518b07..ac36f19 100644
--- a/apps/workbench/test/controllers/pipeline_instances_controller_test.rb
+++ b/apps/workbench/test/controllers/pipeline_instances_controller_test.rb
@@ -3,39 +3,158 @@ require 'test_helper'
 class PipelineInstancesControllerTest < ActionController::TestCase
   include PipelineInstancesHelper
 
-  test "one" do
-    r = [{started_at: 1, finished_at: 3}]
-    assert_equal 2, determine_wallclock_runtime(r)
-
-    r = [{started_at: 1, finished_at: 5}]
-    assert_equal 4, determine_wallclock_runtime(r)
-
-    r = [{started_at: 1, finished_at: 2}, {started_at: 3, finished_at: 5}]
-    assert_equal 3, determine_wallclock_runtime(r)
+  def create_instance_long_enough_to(instance_attrs={})
+    # create 'two_part' pipeline with the given instance attributes
+    pt_fixture = api_fixture('pipeline_templates')['two_part']
+    post :create, {
+      pipeline_instance: instance_attrs.merge({
+        pipeline_template_uuid: pt_fixture['uuid']
+      }),
+      format: :json
+    }, session_for(:active)
+    assert_response :success
+    pi_uuid = assigns(:object).uuid
+    assert_not_nil assigns(:object)
+
+    # yield
+    yield pi_uuid, pt_fixture
+
+    # delete the pipeline instance
+    use_token :active
+    PipelineInstance.where(uuid: pi_uuid).first.destroy
+  end
 
-    r = [{started_at: 3, finished_at: 5}, {started_at: 1, finished_at: 2}]
-    assert_equal 3, determine_wallclock_runtime(r)
+  test "pipeline instance components populated after create" do
+    create_instance_long_enough_to do |new_instance_uuid, template_fixture|
+      assert_equal(template_fixture['components'].to_json,
+                   assigns(:object).components.to_json)
+    end
+  end
 
-    r = [{started_at: 3, finished_at: 5}, {started_at: 1, finished_at: 2},
-         {started_at: 2, finished_at: 4}]
-    assert_equal 4, determine_wallclock_runtime(r)
+  test "can render pipeline instance with tagged collections" do
+    # Make sure to pass in a tagged collection to test that part of the rendering behavior.
+    get(:show,
+        {id: api_fixture("pipeline_instances")["pipeline_with_tagged_collection_input"]["uuid"]},
+        session_for(:active))
+    assert_response :success
+  end
 
-    r = [{started_at: 1, finished_at: 5}, {started_at: 2, finished_at: 3}]
-    assert_equal 4, determine_wallclock_runtime(r)
+  test "update script_parameters one at a time using merge param" do
+      template_fixture = api_fixture('pipeline_templates')['two_part']
+      post :update, {
+        id: api_fixture("pipeline_instances")["pipeline_to_merge_params"]["uuid"],
+        pipeline_instance: {
+          components: {
+            "part-two" => {
+              script_parameters: {
+                integer_with_value: {
+                  value: 9
+                },
+                plain_string: {
+                  value: 'quux'
+                },
+              }
+            }
+          }
+        },
+        merge: true,
+        format: :json
+      }, session_for(:active)
+      assert_response :success
+      assert_not_nil assigns(:object)
+      orig_params = template_fixture['components']['part-two']['script_parameters']
+      new_params = assigns(:object).components[:'part-two'][:script_parameters]
+      orig_params.keys.each do |k|
+        unless %w(integer_with_value plain_string).index(k)
+          assert_equal orig_params[k].to_json, new_params[k.to_sym].to_json
+        end
+      end
+  end
 
-    r = [{started_at: 3, finished_at: 5}, {started_at: 1, finished_at: 4}]
-    assert_equal 4, determine_wallclock_runtime(r)
+  test "component rendering copes with unexpected components format" do
+    get(:show,
+        {id: api_fixture("pipeline_instances")["components_is_jobspec"]["uuid"]},
+        session_for(:active))
+    assert_response :success
+  end
 
-    r = [{started_at: 1, finished_at: 4}, {started_at: 3, finished_at: 5}]
-    assert_equal 4, determine_wallclock_runtime(r)
+  test "dates in JSON components are parsed" do
+    get(:show,
+        {id: api_fixture('pipeline_instances')['has_component_with_completed_jobs']['uuid']},
+        session_for(:active))
+    assert_response :success
+    assert_not_nil assigns(:object)
+    assert_not_nil assigns(:object).components[:foo][:job]
+    assert assigns(:object).components[:foo][:job][:started_at].is_a? Time
+    assert assigns(:object).components[:foo][:job][:finished_at].is_a? Time
+  end
 
-    r = [{started_at: 1, finished_at: 4}, {started_at: 3, finished_at: 5},
-         {started_at: 5, finished_at: 8}]
-    assert_equal 7, determine_wallclock_runtime(r)
+  # The next two tests ensure that a pipeline instance can be copied
+  # when the template has components that do not exist in the
+  # instance (ticket #4000).
+
+  test "copy pipeline instance with components=use_latest" do
+    post(:copy,
+         {
+           id: api_fixture('pipeline_instances')['pipeline_with_newer_template']['uuid'],
+           components: 'use_latest',
+           script: 'use_latest',
+           pipeline_instance: {
+             state: 'RunningOnServer'
+           }
+         },
+         session_for(:active))
+    assert_response 302
+    assert_not_nil assigns(:object)
+
+    # Component 'foo' has script parameters only in the pipeline instance.
+    # Component 'bar' is present only in the pipeline_template.
+    # Test that the copied pipeline instance includes parameters for
+    # component 'foo' from the source instance, and parameters for
+    # component 'bar' from the source template.
+    #
+    assert_not_nil assigns(:object).components[:foo]
+    foo = assigns(:object).components[:foo]
+    assert_not_nil foo[:script_parameters]
+    assert_not_nil foo[:script_parameters][:input]
+    assert_equal 'foo instance input', foo[:script_parameters][:input][:title]
+
+    assert_not_nil assigns(:object).components[:bar]
+    bar = assigns(:object).components[:bar]
+    assert_not_nil bar[:script_parameters]
+    assert_not_nil bar[:script_parameters][:input]
+    assert_equal 'bar template input', bar[:script_parameters][:input][:title]
+  end
 
-    r = [{started_at: 1, finished_at: 4}, {started_at: 3, finished_at: 5},
-         {started_at: 6, finished_at: 8}]
-    assert_equal 6, determine_wallclock_runtime(r)
+  test "copy pipeline instance on newer template works with script=use_same" do
+    post(:copy,
+         {
+           id: api_fixture('pipeline_instances')['pipeline_with_newer_template']['uuid'],
+           components: 'use_latest',
+           script: 'use_same',
+           pipeline_instance: {
+             state: 'RunningOnServer'
+           }
+         },
+         session_for(:active))
+    assert_response 302
+    assert_not_nil assigns(:object)
+
+    # Test that relevant component parameters were copied from both
+    # the source instance and source template, respectively (see
+    # previous test)
+    #
+    assert_not_nil assigns(:object).components[:foo]
+    foo = assigns(:object).components[:foo]
+    assert_not_nil foo[:script_parameters]
+    assert_not_nil foo[:script_parameters][:input]
+    assert_equal 'foo instance input', foo[:script_parameters][:input][:title]
+
+    assert_not_nil assigns(:object).components[:bar]
+    bar = assigns(:object).components[:bar]
+    assert_not_nil bar[:script_parameters]
+    assert_not_nil bar[:script_parameters][:input]
+    assert_equal 'bar template input', bar[:script_parameters][:input][:title]
   end
 
   test "generate graph" do
diff --git a/apps/workbench/test/functional/pipeline_templates_controller_test.rb b/apps/workbench/test/controllers/pipeline_templates_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/pipeline_templates_controller_test.rb
rename to apps/workbench/test/controllers/pipeline_templates_controller_test.rb
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/controllers/projects_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/projects_controller_test.rb
rename to apps/workbench/test/controllers/projects_controller_test.rb
diff --git a/apps/workbench/test/functional/repositories_controller_test.rb b/apps/workbench/test/controllers/repositories_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/repositories_controller_test.rb
rename to apps/workbench/test/controllers/repositories_controller_test.rb
diff --git a/apps/workbench/test/functional/sessions_controller_test.rb b/apps/workbench/test/controllers/sessions_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/sessions_controller_test.rb
rename to apps/workbench/test/controllers/sessions_controller_test.rb
diff --git a/apps/workbench/test/functional/specimens_controller_test.rb b/apps/workbench/test/controllers/specimens_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/specimens_controller_test.rb
rename to apps/workbench/test/controllers/specimens_controller_test.rb
diff --git a/apps/workbench/test/functional/traits_controller_test.rb b/apps/workbench/test/controllers/traits_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/traits_controller_test.rb
rename to apps/workbench/test/controllers/traits_controller_test.rb
diff --git a/apps/workbench/test/functional/user_agreements_controller_test.rb b/apps/workbench/test/controllers/user_agreements_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/user_agreements_controller_test.rb
rename to apps/workbench/test/controllers/user_agreements_controller_test.rb
diff --git a/apps/workbench/test/functional/users_controller_test.rb b/apps/workbench/test/controllers/users_controller_test.rb
similarity index 96%
rename from apps/workbench/test/functional/users_controller_test.rb
rename to apps/workbench/test/controllers/users_controller_test.rb
index a734391..213a2a5 100644
--- a/apps/workbench/test/functional/users_controller_test.rb
+++ b/apps/workbench/test/controllers/users_controller_test.rb
@@ -1,7 +1,7 @@
 require 'test_helper'
 
 class UsersControllerTest < ActionController::TestCase
-  test "valid token works in functional test" do
+  test "valid token works in controller test" do
     get :index, {}, session_for(:active)
     assert_response :success
   end
diff --git a/apps/workbench/test/functional/virtual_machines_controller_test.rb b/apps/workbench/test/controllers/virtual_machines_controller_test.rb
similarity index 100%
rename from apps/workbench/test/functional/virtual_machines_controller_test.rb
rename to apps/workbench/test/controllers/virtual_machines_controller_test.rb
diff --git a/apps/workbench/test/functional/.gitkeep b/apps/workbench/test/functional/.gitkeep
deleted file mode 100644
index e69de29..0000000
diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
deleted file mode 100644
index 7c406c2..0000000
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ /dev/null
@@ -1,234 +0,0 @@
-require 'test_helper'
-
-class CollectionsControllerTest < ActionController::TestCase
-  # These tests don't do state-changing API calls. Save some time by
-  # skipping the database reset.
-  reset_api_fixtures :after_each_test, false
-  reset_api_fixtures :after_suite, true
-
-  NONEXISTENT_COLLECTION = "ffffffffffffffffffffffffffffffff+0"
-
-  def stub_file_content
-    # For the duration of the current test case, stub file download
-    # content with a randomized (but recognizable) string. Return the
-    # string, the test case can use it in assertions.
-    txt = 'the quick brown fox ' + rand(2**32).to_s
-    @controller.stubs(:file_enumerator).returns([txt])
-    txt
-  end
-
-  def collection_params(collection_name, file_name=nil)
-    uuid = api_fixture('collections')[collection_name.to_s]['uuid']
-    params = {uuid: uuid, id: uuid}
-    params[:file] = file_name if file_name
-    params
-  end
-
-  def assert_hash_includes(actual_hash, expected_hash, msg=nil)
-    expected_hash.each do |key, value|
-      assert_equal(value, actual_hash[key], msg)
-    end
-  end
-
-  def assert_no_session
-    assert_hash_includes(session, {arvados_api_token: nil},
-                         "session includes unexpected API token")
-  end
-
-  def assert_session_for_auth(client_auth)
-    api_token =
-      api_fixture('api_client_authorizations')[client_auth.to_s]['api_token']
-    assert_hash_includes(session, {arvados_api_token: api_token},
-                         "session token does not belong to #{client_auth}")
-  end
-
-  def show_collection(params, session={}, response=:success)
-    params = collection_params(params) if not params.is_a? Hash
-    session = session_for(session) if not session.is_a? Hash
-    get(:show, params, session)
-    assert_response response
-  end
-
-  test "viewing a collection" do
-    show_collection(:foo_file, :active)
-    assert_equal([['.', 'foo', 3]], assigns(:object).files)
-  end
-
-  test "viewing a collection fetches related projects" do
-    show_collection({id: api_fixture('collections')["foo_file"]['portable_data_hash']}, :active)
-    assert_includes(assigns(:same_pdh).map(&:owner_uuid),
-                    api_fixture('groups')['aproject']['uuid'],
-                    "controller did not find linked project")
-  end
-
-  test "viewing a collection fetches related permissions" do
-    show_collection(:bar_file, :active)
-    assert_includes(assigns(:permissions).map(&:uuid),
-                    api_fixture('links')['bar_file_readable_by_active']['uuid'],
-                    "controller did not find permission link")
-  end
-
-  test "viewing a collection fetches jobs that output it" do
-    show_collection(:bar_file, :active)
-    assert_includes(assigns(:output_of).map(&:uuid),
-                    api_fixture('jobs')['foobar']['uuid'],
-                    "controller did not find output job")
-  end
-
-  test "viewing a collection fetches jobs that logged it" do
-    show_collection(:baz_file, :active)
-    assert_includes(assigns(:log_of).map(&:uuid),
-                    api_fixture('jobs')['foobar']['uuid'],
-                    "controller did not find logger job")
-  end
-
-  test "viewing a collection fetches logs about it" do
-    show_collection(:foo_file, :active)
-    assert_includes(assigns(:logs).map(&:uuid),
-                    api_fixture('logs')['log4']['uuid'],
-                    "controller did not find related log")
-  end
-
-  test "viewing collection files with a reader token" do
-    params = collection_params(:foo_file)
-    params[:reader_token] = api_fixture("api_client_authorizations",
-                                        "active_all_collections", "api_token")
-    get(:show_file_links, params)
-    assert_response :success
-    assert_equal([['.', 'foo', 3]], assigns(:object).files)
-    assert_no_session
-  end
-
-  test "fetching collection file with reader token" do
-    expected = stub_file_content
-    params = collection_params(:foo_file, "foo")
-    params[:reader_token] = api_fixture("api_client_authorizations",
-                                        "active_all_collections", "api_token")
-    get(:show_file, params)
-    assert_response :success
-    assert_equal(expected, @response.body,
-                 "failed to fetch a Collection file with a reader token")
-    assert_no_session
-  end
-
-  test "reader token Collection links end with trailing slash" do
-    # Testing the fix for #2937.
-    session = session_for(:active_trustedclient)
-    post(:share, collection_params(:foo_file), session)
-    assert(@controller.download_link.ends_with? '/',
-           "Collection share link does not end with slash for wget")
-  end
-
-  test "getting a file from Keep" do
-    params = collection_params(:foo_file, 'foo')
-    sess = session_for(:active)
-    expect_content = stub_file_content
-    get(:show_file, params, sess)
-    assert_response :success
-    assert_equal(expect_content, @response.body,
-                 "failed to get a correct file from Keep")
-  end
-
-  test "can't get a file from Keep without permission" do
-    params = collection_params(:foo_file, 'foo')
-    sess = session_for(:spectator)
-    get(:show_file, params, sess)
-    assert_response 404
-  end
-
-  test "trying to get a nonexistent file from Keep returns a 404" do
-    params = collection_params(:foo_file, 'gone')
-    sess = session_for(:admin)
-    get(:show_file, params, sess)
-    assert_response 404
-  end
-
-  test "getting a file from Keep with a good reader token" do
-    params = collection_params(:foo_file, 'foo')
-    read_token = api_fixture('api_client_authorizations')['active']['api_token']
-    params[:reader_token] = read_token
-    expect_content = stub_file_content
-    get(:show_file, params)
-    assert_response :success
-    assert_equal(expect_content, @response.body,
-                 "failed to get a correct file from Keep using a reader token")
-    assert_not_equal(read_token, session[:arvados_api_token],
-                     "using a reader token set the session's API token")
-  end
-
-  test "trying to get from Keep with an unscoped reader token prompts login" do
-    params = collection_params(:foo_file, 'foo')
-    params[:reader_token] =
-      api_fixture('api_client_authorizations')['active_noscope']['api_token']
-    get(:show_file, params)
-    assert_response :redirect
-  end
-
-  test "can get a file with an unpermissioned auth but in-scope reader token" do
-    params = collection_params(:foo_file, 'foo')
-    sess = session_for(:expired)
-    read_token = api_fixture('api_client_authorizations')['active']['api_token']
-    params[:reader_token] = read_token
-    expect_content = stub_file_content
-    get(:show_file, params, sess)
-    assert_response :success
-    assert_equal(expect_content, @response.body,
-                 "failed to get a correct file from Keep using a reader token")
-    assert_not_equal(read_token, session[:arvados_api_token],
-                     "using a reader token set the session's API token")
-  end
-
-  test "inactive user can retrieve user agreement" do
-    ua_collection = api_fixture('collections')['user_agreement']
-    # Here we don't test whether the agreement can be retrieved from
-    # Keep. We only test that show_file decides to send file content,
-    # so we use the file content stub.
-    stub_file_content
-    get :show_file, {
-      uuid: ua_collection['uuid'],
-      file: ua_collection['manifest_text'].match(/ \d+:\d+:(\S+)/)[1]
-    }, session_for(:inactive)
-    assert_nil(assigns(:unsigned_user_agreements),
-               "Did not skip check_user_agreements filter " +
-               "when showing the user agreement.")
-    assert_response :success
-  end
-
-  test "requesting nonexistent Collection returns 404" do
-    show_collection({uuid: NONEXISTENT_COLLECTION, id: NONEXISTENT_COLLECTION},
-                    :active, 404)
-  end
-
-  test "use a reasonable read buffer even if client requests a huge range" do
-    fakefiledata = mock
-    IO.expects(:popen).returns(fakefiledata)
-    fakefiledata.expects(:read).twice.with() do |length|
-      # Fail the test if read() is called with length>1MiB:
-      length < 2**20
-      ## Force the ActionController::Live thread to lose the race to
-      ## verify that @response.body.length actually waits for the
-      ## response (see below):
-      # sleep 3
-    end.returns("foo\n", nil)
-    fakefiledata.expects(:close)
-    foo_file = api_fixture('collections')['foo_file']
-    @request.headers['Range'] = 'bytes=0-4294967296/*'
-    get :show_file, {
-      uuid: foo_file['uuid'],
-      file: foo_file['manifest_text'].match(/ \d+:\d+:(\S+)/)[1]
-    }, session_for(:active)
-    # Wait for the whole response to arrive before deciding whether
-    # mocks' expectations were met. Otherwise, Mocha will fail the
-    # test depending on how slowly the ActionController::Live thread
-    # runs.
-    @response.body.length
-  end
-
-  test "show file in a subdirectory of a collection" do
-    params = collection_params(:collection_with_files_in_subdir, 'subdir2/subdir3/subdir4/file1_in_subdir4.txt')
-    expect_content = stub_file_content
-    get(:show_file, params, session_for(:user1_with_load))
-    assert_response :success
-    assert_equal(expect_content, @response.body, "failed to get a correct file from Keep")
-  end
-end
diff --git a/apps/workbench/test/functional/pipeline_instances_controller_test.rb b/apps/workbench/test/functional/pipeline_instances_controller_test.rb
deleted file mode 100644
index a14d419..0000000
--- a/apps/workbench/test/functional/pipeline_instances_controller_test.rb
+++ /dev/null
@@ -1,157 +0,0 @@
-require 'test_helper'
-
-class PipelineInstancesControllerTest < ActionController::TestCase
-  def create_instance_long_enough_to(instance_attrs={})
-    # create 'two_part' pipeline with the given instance attributes
-    pt_fixture = api_fixture('pipeline_templates')['two_part']
-    post :create, {
-      pipeline_instance: instance_attrs.merge({
-        pipeline_template_uuid: pt_fixture['uuid']
-      }),
-      format: :json
-    }, session_for(:active)
-    assert_response :success
-    pi_uuid = assigns(:object).uuid
-    assert_not_nil assigns(:object)
-
-    # yield
-    yield pi_uuid, pt_fixture
-
-    # delete the pipeline instance
-    use_token :active
-    PipelineInstance.where(uuid: pi_uuid).first.destroy
-  end
-
-  test "pipeline instance components populated after create" do
-    create_instance_long_enough_to do |new_instance_uuid, template_fixture|
-      assert_equal(template_fixture['components'].to_json,
-                   assigns(:object).components.to_json)
-    end
-  end
-
-  test "can render pipeline instance with tagged collections" do
-    # Make sure to pass in a tagged collection to test that part of the rendering behavior.
-    get(:show,
-        {id: api_fixture("pipeline_instances")["pipeline_with_tagged_collection_input"]["uuid"]},
-        session_for(:active))
-    assert_response :success
-  end
-
-  test "update script_parameters one at a time using merge param" do
-      template_fixture = api_fixture('pipeline_templates')['two_part']
-      post :update, {
-        id: api_fixture("pipeline_instances")["pipeline_to_merge_params"]["uuid"],
-        pipeline_instance: {
-          components: {
-            "part-two" => {
-              script_parameters: {
-                integer_with_value: {
-                  value: 9
-                },
-                plain_string: {
-                  value: 'quux'
-                },
-              }
-            }
-          }
-        },
-        merge: true,
-        format: :json
-      }, session_for(:active)
-      assert_response :success
-      assert_not_nil assigns(:object)
-      orig_params = template_fixture['components']['part-two']['script_parameters']
-      new_params = assigns(:object).components[:'part-two'][:script_parameters]
-      orig_params.keys.each do |k|
-        unless %w(integer_with_value plain_string).index(k)
-          assert_equal orig_params[k].to_json, new_params[k.to_sym].to_json
-        end
-      end
-  end
-
-  test "component rendering copes with unexpected components format" do
-    get(:show,
-        {id: api_fixture("pipeline_instances")["components_is_jobspec"]["uuid"]},
-        session_for(:active))
-    assert_response :success
-  end
-
-  test "dates in JSON components are parsed" do
-    get(:show,
-        {id: api_fixture('pipeline_instances')['has_component_with_completed_jobs']['uuid']},
-        session_for(:active))
-    assert_response :success
-    assert_not_nil assigns(:object)
-    assert_not_nil assigns(:object).components[:foo][:job]
-    assert assigns(:object).components[:foo][:job][:started_at].is_a? Time
-    assert assigns(:object).components[:foo][:job][:finished_at].is_a? Time
-  end
-
-  # The next two tests ensure that a pipeline instance can be copied
-  # when the template has components that do not exist in the
-  # instance (ticket #4000).
-
-  test "copy pipeline instance with components=use_latest" do
-    post(:copy,
-         {
-           id: api_fixture('pipeline_instances')['pipeline_with_newer_template']['uuid'],
-           components: 'use_latest',
-           script: 'use_latest',
-           pipeline_instance: {
-             state: 'RunningOnServer'
-           }
-         },
-         session_for(:active))
-    assert_response 302
-    assert_not_nil assigns(:object)
-
-    # Component 'foo' has script parameters only in the pipeline instance.
-    # Component 'bar' is present only in the pipeline_template.
-    # Test that the copied pipeline instance includes parameters for
-    # component 'foo' from the source instance, and parameters for
-    # component 'bar' from the source template.
-    #
-    assert_not_nil assigns(:object).components[:foo]
-    foo = assigns(:object).components[:foo]
-    assert_not_nil foo[:script_parameters]
-    assert_not_nil foo[:script_parameters][:input]
-    assert_equal 'foo instance input', foo[:script_parameters][:input][:title]
-
-    assert_not_nil assigns(:object).components[:bar]
-    bar = assigns(:object).components[:bar]
-    assert_not_nil bar[:script_parameters]
-    assert_not_nil bar[:script_parameters][:input]
-    assert_equal 'bar template input', bar[:script_parameters][:input][:title]
-  end
-
-  test "copy pipeline instance on newer template works with script=use_same" do
-    post(:copy,
-         {
-           id: api_fixture('pipeline_instances')['pipeline_with_newer_template']['uuid'],
-           components: 'use_latest',
-           script: 'use_same',
-           pipeline_instance: {
-             state: 'RunningOnServer'
-           }
-         },
-         session_for(:active))
-    assert_response 302
-    assert_not_nil assigns(:object)
-
-    # Test that relevant component parameters were copied from both
-    # the source instance and source template, respectively (see
-    # previous test)
-    #
-    assert_not_nil assigns(:object).components[:foo]
-    foo = assigns(:object).components[:foo]
-    assert_not_nil foo[:script_parameters]
-    assert_not_nil foo[:script_parameters][:input]
-    assert_equal 'foo instance input', foo[:script_parameters][:input][:title]
-
-    assert_not_nil assigns(:object).components[:bar]
-    bar = assigns(:object).components[:bar]
-    assert_not_nil bar[:script_parameters]
-    assert_not_nil bar[:script_parameters][:input]
-    assert_equal 'bar template input', bar[:script_parameters][:input][:title]
-  end
-end
diff --git a/apps/workbench/test/helpers/pipeline_instances_helper_test.rb b/apps/workbench/test/helpers/pipeline_instances_helper_test.rb
new file mode 100644
index 0000000..a785683
--- /dev/null
+++ b/apps/workbench/test/helpers/pipeline_instances_helper_test.rb
@@ -0,0 +1,38 @@
+require 'test_helper'
+
+class PipelineInstancesHelperTest < ActionView::TestCase
+  test "one" do
+    r = [{started_at: 1, finished_at: 3}]
+    assert_equal 2, determine_wallclock_runtime(r)
+
+    r = [{started_at: 1, finished_at: 5}]
+    assert_equal 4, determine_wallclock_runtime(r)
+
+    r = [{started_at: 1, finished_at: 2}, {started_at: 3, finished_at: 5}]
+    assert_equal 3, determine_wallclock_runtime(r)
+
+    r = [{started_at: 3, finished_at: 5}, {started_at: 1, finished_at: 2}]
+    assert_equal 3, determine_wallclock_runtime(r)
+
+    r = [{started_at: 3, finished_at: 5}, {started_at: 1, finished_at: 2},
+         {started_at: 2, finished_at: 4}]
+    assert_equal 4, determine_wallclock_runtime(r)
+
+    r = [{started_at: 1, finished_at: 5}, {started_at: 2, finished_at: 3}]
+    assert_equal 4, determine_wallclock_runtime(r)
+
+    r = [{started_at: 3, finished_at: 5}, {started_at: 1, finished_at: 4}]
+    assert_equal 4, determine_wallclock_runtime(r)
+
+    r = [{started_at: 1, finished_at: 4}, {started_at: 3, finished_at: 5}]
+    assert_equal 4, determine_wallclock_runtime(r)
+
+    r = [{started_at: 1, finished_at: 4}, {started_at: 3, finished_at: 5},
+         {started_at: 5, finished_at: 8}]
+    assert_equal 7, determine_wallclock_runtime(r)
+
+    r = [{started_at: 1, finished_at: 4}, {started_at: 3, finished_at: 5},
+         {started_at: 6, finished_at: 8}]
+    assert_equal 6, determine_wallclock_runtime(r)
+  end
+end
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 58e540c..dbeb9aa 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -192,7 +192,7 @@ class ActionController::TestCase
   def check_counter action
     @counter += 1
     if @counter == 2
-      assert_equal 1, 2, "Multiple actions in functional test"
+      assert_equal 1, 2, "Multiple actions in controller test"
     end
   end
 

commit 7e828d4672d2b306bccacfed309123c81846d565
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Nov 26 19:46:27 2014 -0500

    4533: Skip one-fixture-reset-per-test in a few test classes.

diff --git a/apps/workbench/test/functional/application_controller_test.rb b/apps/workbench/test/functional/application_controller_test.rb
index c282802..d0d9c5d 100644
--- a/apps/workbench/test/functional/application_controller_test.rb
+++ b/apps/workbench/test/functional/application_controller_test.rb
@@ -1,6 +1,10 @@
 require 'test_helper'
 
 class ApplicationControllerTest < ActionController::TestCase
+  # These tests don't do state-changing API calls. Save some time by
+  # skipping the database reset.
+  reset_api_fixtures :after_each_test, false
+  reset_api_fixtures :after_suite, true
 
   setup do
     @user_dataclass = ArvadosBase.resource_class_for_uuid(api_fixture('users')['active']['uuid'])
diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
index 6c64ac9..7c406c2 100644
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ b/apps/workbench/test/functional/collections_controller_test.rb
@@ -1,6 +1,11 @@
 require 'test_helper'
 
 class CollectionsControllerTest < ActionController::TestCase
+  # These tests don't do state-changing API calls. Save some time by
+  # skipping the database reset.
+  reset_api_fixtures :after_each_test, false
+  reset_api_fixtures :after_suite, true
+
   NONEXISTENT_COLLECTION = "ffffffffffffffffffffffffffffffff+0"
 
   def stub_file_content
diff --git a/apps/workbench/test/integration/application_layout_test.rb b/apps/workbench/test/integration/application_layout_test.rb
index 69e346d..0939159 100644
--- a/apps/workbench/test/integration/application_layout_test.rb
+++ b/apps/workbench/test/integration/application_layout_test.rb
@@ -3,6 +3,11 @@ require 'selenium-webdriver'
 require 'headless'
 
 class ApplicationLayoutTest < ActionDispatch::IntegrationTest
+  # These tests don't do state-changing API calls. Save some time by
+  # skipping the database reset.
+  reset_api_fixtures :after_each_test, false
+  reset_api_fixtures :after_suite, true
+
   setup do
     headless = Headless.new
     headless.start

commit 437d4225617df22b4e6d9c245f854620cdc09bc7
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Nov 26 19:38:48 2014 -0500

    4533: Disable database-reset for all diagnostics test classes.

diff --git a/apps/workbench/test/diagnostics/pipeline_test.rb b/apps/workbench/test/diagnostics/pipeline_test.rb
index e69f86d..fbc144a 100644
--- a/apps/workbench/test/diagnostics/pipeline_test.rb
+++ b/apps/workbench/test/diagnostics/pipeline_test.rb
@@ -3,10 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class PipelineTest < DiagnosticsTest
-  reset_api_fixtures :after_each_test, false
-  reset_api_fixtures :after_suite, false
-  reset_api_fixtures :before_suite, false
-
   pipelines_to_test = Rails.configuration.pipelines_to_test.andand.keys
 
   setup do
diff --git a/apps/workbench/test/diagnostics_test_helper.rb b/apps/workbench/test/diagnostics_test_helper.rb
index 01d351a..e4a249e 100644
--- a/apps/workbench/test/diagnostics_test_helper.rb
+++ b/apps/workbench/test/diagnostics_test_helper.rb
@@ -26,4 +26,8 @@ class DiagnosticsTest < ActionDispatch::IntegrationTest
     end
   end
 
+  # Diagnostics tests never want to reset the database fixtures.
+  protected
+  def self.reset_api_fixtures_now end
+
 end

commit e3914c4981a48d086562bcdc45f0d385f95fc083
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Nov 26 19:34:13 2014 -0500

    4533: Move "database reset does not break basic operations" into its own test.

diff --git a/services/api/test/integration/remote_reset_test.rb b/services/api/test/integration/remote_reset_test.rb
index 9cd9d0f..b3a7c63 100644
--- a/services/api/test/integration/remote_reset_test.rb
+++ b/services/api/test/integration/remote_reset_test.rb
@@ -3,42 +3,55 @@ require 'test_helper'
 class RemoteResetTest < ActionDispatch::IntegrationTest
   self.use_transactional_fixtures = false
 
-  test "roll back database change" do
+  test "database reset doesn't break basic CRUD operations" do
     active_auth = auth(:active)
     admin_auth = auth(:admin)
 
-    old_uuid = specimens(:owned_by_active_user).uuid
-    new_uuid = nil
     authorize_with :admin
     post '/database/reset', {}, admin_auth
     assert_response :success
 
-    delete '/arvados/v1/specimens/' + old_uuid, {}, active_auth
-    assert_response :success
     post '/arvados/v1/specimens', {specimen: '{}'}, active_auth
     assert_response :success
     new_uuid = json_response['uuid']
 
-    # Perhaps redundant: confirm the above changes are observable from
-    # here using get(). Otherwise, the assertions below would not be
-    # informative. (Aside from confirming that integration-testing
-    # features work, this confirms that /database/reset didn't somehow
-    # put the database into a "don't persist any changes" mode -- not
-    # that I can think of a way for that to happen.)
     get '/arvados/v1/specimens/'+new_uuid, {}, active_auth
     assert_response :success
-    get '/arvados/v1/specimens/'+old_uuid, {}, active_auth
+
+    put('/arvados/v1/specimens/'+new_uuid,
+        {specimen: '{"properties":{}}'}, active_auth)
+    assert_response :success
+
+    delete '/arvados/v1/specimens/'+new_uuid, {}, active_auth
+    assert_response :success
+
+    get '/arvados/v1/specimens/'+new_uuid, {}, active_auth
     assert_response 404
+  end
+
+  test "roll back database change" do
+    active_auth = auth(:active)
+    admin_auth = auth(:admin)
+
+    old_uuid = specimens(:owned_by_active_user).uuid
+    authorize_with :admin
+    post '/database/reset', {}, admin_auth
+    assert_response :success
+
+    delete '/arvados/v1/specimens/' + old_uuid, {}, active_auth
+    assert_response :success
+    post '/arvados/v1/specimens', {specimen: '{}'}, active_auth
+    assert_response :success
+    new_uuid = json_response['uuid']
 
     # Reset to fixtures.
     post '/database/reset', {}, admin_auth
     assert_response :success
 
-    # New speciment should disappear. Old specimen should reappear.
+    # New specimen should disappear. Old specimen should reappear.
     get '/arvados/v1/specimens/'+new_uuid, {}, active_auth
     assert_response 404
     get '/arvados/v1/specimens/'+old_uuid, {}, active_auth
     assert_response :success
-    assert_empty Specimen.where(uuid: new_uuid)
   end
 end

commit a69a1cb084e6feb9e4e9c6538a52289e7c56700b
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Nov 26 18:58:58 2014 -0500

    4533: Reset fixtures after each test case by default.

diff --git a/apps/workbench/test/diagnostics/pipeline_test.rb b/apps/workbench/test/diagnostics/pipeline_test.rb
index fbc144a..e69f86d 100644
--- a/apps/workbench/test/diagnostics/pipeline_test.rb
+++ b/apps/workbench/test/diagnostics/pipeline_test.rb
@@ -3,6 +3,10 @@ require 'selenium-webdriver'
 require 'headless'
 
 class PipelineTest < DiagnosticsTest
+  reset_api_fixtures :after_each_test, false
+  reset_api_fixtures :after_suite, false
+  reset_api_fixtures :before_suite, false
+
   pipelines_to_test = Rails.configuration.pipelines_to_test.andand.keys
 
   setup do
diff --git a/apps/workbench/test/integration/collections_test.rb b/apps/workbench/test/integration/collections_test.rb
index 5925cd5..f4fc4cb 100644
--- a/apps/workbench/test/integration/collections_test.rb
+++ b/apps/workbench/test/integration/collections_test.rb
@@ -3,8 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class CollectionsTest < ActionDispatch::IntegrationTest
-  reset_api_fixtures :after_suite
-
   setup do
     Capybara.current_driver = :rack_test
   end
diff --git a/apps/workbench/test/integration/pipeline_instances_test.rb b/apps/workbench/test/integration/pipeline_instances_test.rb
index 32f7bf3..7e3696c 100644
--- a/apps/workbench/test/integration/pipeline_instances_test.rb
+++ b/apps/workbench/test/integration/pipeline_instances_test.rb
@@ -3,8 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class PipelineInstancesTest < ActionDispatch::IntegrationTest
-  reset_api_fixtures :after_suite
-
   setup do
     # Selecting collections requiresLocalStorage
     headless = Headless.new
diff --git a/apps/workbench/test/integration/projects_test.rb b/apps/workbench/test/integration/projects_test.rb
index bbc308e..0c6e5bc 100644
--- a/apps/workbench/test/integration/projects_test.rb
+++ b/apps/workbench/test/integration/projects_test.rb
@@ -3,8 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class ProjectsTest < ActionDispatch::IntegrationTest
-  reset_api_fixtures :after_suite
-
   setup do
     headless = Headless.new
     headless.start
@@ -287,7 +285,7 @@ class ProjectsTest < ActionDispatch::IntegrationTest
     ['Remove',api_fixture('collections')['collection_in_aproject_with_same_name_as_in_home_project'],
       api_fixture('groups')['aproject'],nil,true],
   ].each do |action, my_collection, src, dest=nil, expect_name_change=nil|
-    test "selection #{action} #{expect_name_change} for project" do
+    test "selection #{action} -> #{expect_name_change.inspect} for project" do
       perform_selection_action src, dest, my_collection, action
 
       case action
@@ -298,8 +296,6 @@ class ProjectsTest < ActionDispatch::IntegrationTest
         find(".dropdown-menu a", text: dest['name']).click
         assert page.has_text?(my_collection['name']), 'Collection not found in dest project after copy'
 
-        # now remove it from destination project to restore to original state
-        perform_selection_action dest, nil, my_collection, 'Remove'
       when 'Move'
         assert page.has_no_text?(my_collection['name']), 'Collection still found in src project after move'
         visit page_with_token 'active', '/'
@@ -307,8 +303,6 @@ class ProjectsTest < ActionDispatch::IntegrationTest
         find(".dropdown-menu a", text: dest['name']).click
         assert page.has_text?(my_collection['name']), 'Collection not found in dest project after move'
 
-        # move it back to src project to restore to original state
-        perform_selection_action dest, src, my_collection, action
       when 'Remove'
         assert page.has_no_text?(my_collection['name']), 'Collection still found in src project after remove'
         visit page_with_token 'active', '/'
diff --git a/apps/workbench/test/integration/user_agreements_test.rb b/apps/workbench/test/integration/user_agreements_test.rb
index acc3bca..dd263a2 100644
--- a/apps/workbench/test/integration/user_agreements_test.rb
+++ b/apps/workbench/test/integration/user_agreements_test.rb
@@ -3,8 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class UserAgreementsTest < ActionDispatch::IntegrationTest
-  # We might change user activation status here, which can affect other test suites.
-  reset_api_fixtures :after_suite
 
   setup do
     Capybara.current_driver = Capybara.javascript_driver
diff --git a/apps/workbench/test/integration/user_manage_account_test.rb b/apps/workbench/test/integration/user_manage_account_test.rb
index 4f124fc..ff2b7dd 100644
--- a/apps/workbench/test/integration/user_manage_account_test.rb
+++ b/apps/workbench/test/integration/user_manage_account_test.rb
@@ -3,8 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class UserManageAccountTest < ActionDispatch::IntegrationTest
-  reset_api_fixtures :after_suite
-
   setup do
     headless = Headless.new
     headless.start
diff --git a/apps/workbench/test/integration/user_profile_test.rb b/apps/workbench/test/integration/user_profile_test.rb
index e79a132..fd190a2 100644
--- a/apps/workbench/test/integration/user_profile_test.rb
+++ b/apps/workbench/test/integration/user_profile_test.rb
@@ -3,8 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class UserProfileTest < ActionDispatch::IntegrationTest
-  reset_api_fixtures :after_suite
-
   setup do
     headless = Headless.new
     headless.start
diff --git a/apps/workbench/test/integration/users_test.rb b/apps/workbench/test/integration/users_test.rb
index 00a25ba..58432f7 100644
--- a/apps/workbench/test/integration/users_test.rb
+++ b/apps/workbench/test/integration/users_test.rb
@@ -3,7 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class UsersTest < ActionDispatch::IntegrationTest
-  reset_api_fixtures :after_suite
 
   test "login as active user but not admin" do
     Capybara.current_driver = Capybara.javascript_driver
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 4fd5aaf..58e540c 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -204,24 +204,59 @@ class ActionController::TestCase
   end
 end
 
-# Test classes can call reset_api_fixtures(:before_suite) or
-# ...(:after_suite)
+# Test classes can call reset_api_fixtures(when_to_reset,flag) to
+# override the default. Example:
+#
+# class MySuite < ActionDispatch::IntegrationTest
+#   reset_api_fixtures :after_each_test, false
+#   reset_api_fixtures :after_suite, true
+#   ...
+# end
+#
+# The default behavior is reset_api_fixtures(:after_each_test,true).
+#
 class ActiveSupport::TestCase
-  class << self
-    attr_accessor :want_reset_api_fixtures
+
+  def self.inherited subclass
+    subclass.class_eval do
+      class << self
+        attr_accessor :want_reset_api_fixtures
+      end
+      @want_reset_api_fixtures = {
+        after_each_test: true,
+        after_suite: false,
+        before_suite: false,
+      }
+    end
+    super
+  end
+  # Existing subclasses of ActiveSupport::TestCase (ones that already
+  # existed before we set up the self.inherited hook above) will not
+  # get their own instance variable. They're not real test cases
+  # anyway, so we give them a "don't reset anywhere" stub.
+  def self.want_reset_api_fixtures
+    {}
   end
 
   def self.reset_api_fixtures where, t=true
-    raise unless [:before_suite, :after_suite].include? where
-    self.want_reset_api_fixtures ||= {}
+    if not want_reset_api_fixtures.has_key? where
+      raise ArgumentError, "There is no #{where.inspect} hook"
+    end
     self.want_reset_api_fixtures[where] = t
   end
 
   def self.run *args
-    self.want_reset_api_fixtures ||= {}
     reset_api_fixtures_now if want_reset_api_fixtures[:before_suite]
-    super
+    result = super
     reset_api_fixtures_now if want_reset_api_fixtures[:after_suite]
+    result
+  end
+
+  def after_teardown
+    if self.class.want_reset_api_fixtures[:after_each_test]
+      self.class.reset_api_fixtures_now
+    end
+    super
   end
 
   protected

commit 76eaaac9fed4e74b8073ee08f99b84fc7922ac9f
Merge: e65d698 84efd06
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Nov 26 12:38:01 2014 -0500

    4533: Merge branch 'master' into 4533-remote-reset


-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list