Testing Best Practices

High level guidelines

• Keep tests fast.
• Don't use the DB if you don't have to.
• Prefer the FactoryBot build_stubbed method.
• Don't write a feature spec if you're not testing user-facing changes
• Don't rely on order in tests.

Ordering in tests

It is not guaranteed that the order in which you create factories will be the same order in the result.

Examples:

• Testing that a DB query sorts results by created_at or some other timestamp. Don't assume that if you create 2 factories in the test, the first one will always come first. Instead, specify a specific timestamp for each factory to ensure the test will be idempotent.

• Testing that the JSON rendered by the controller has certain attributes for some of the entries. Don't assume that if you create multiple factories, the JSON will return them in the same order. Instead of doing something like this:

create(:hearing)
create(:hearing)
hearings = JSON.parse(response.body)["hearings"]
expect(hearings[0]["judge_last_name"]).to eq "Smith"

do this instead:

first_hearing = create(:hearing)
hearings = JSON.parse(response.body)["hearings"]
first_hearing_result = hearings.select { |hash| hash["id"] == first_hearing.id }
expect(first_hearing_result["judge_last_name"]).to eq "Smith"

Caseflow conventions

Tests that write to the DB

• If a test only needs Postgres, add require "support/database_cleaner" at the top of the file (below the frozen string comment). Additionally, tag each test that needs Postgres with the :postgres tag. For example, if all the tests in a file write to the DB, add the tag like so:

RSpec.describe SomeController, :postgres do
...
end

If the spec type is specified, put the tag before the type hash:

RSpec.describe SomeController, :postgres, type: :controller do
...
end

• If a test needs both Postgres and VACOLS, add require "support/vacols_database_cleaner" at the top of the file (below the frozen string comment). Additionally, tag each test that needs the DB with the :all_dbs tag.
• Avoid VACOLS if you can because it makes tests slower. Currently, we have several tests where the only need for VACOLS is to create the staff factory. If possible, prefer to stub out the logic you need. For example, if the only reason the staff is needed is to check a user's vacols_roles, you can stub that out by doing something like this:

allow(UserRepository).to receive(:user_info_from_vacols).with(user.css_id).and_return(roles: ["attorney"])

• If you're not sure if a test writes to the DB, or to which DB, run rm log/test.log, then run the test, then look for any INSERT statements in test.log.

Factories

• Default factories should only contain the minimum amount of attributes required. Create new factories for different scenarios. If some attribute or association doesn't apply globally, don't set it on the default factory.
• Don't create associations with a block in factories. Example:

factory :task do
  appeal { create(:legacy_appeal, vacols_case: create(:case)) }
end

Instead, use the documented way of setting up associations:

# This will use the `appeal` factory
factory :task do
  appeal
end

or if you want a different class and attributes for the association:

factory :legacy_task do
  association :appeal, factory: :legacy_appeal
end

The problem with using create for associations in factories is it prevents the parent factory from being stubbed. For example, if we don't need to use the DB, we can use build_stubbed(:task), but if the task factory defines the appeal association by creating a new appeal, calling build_stubbed(:task) will write to the DB. We can get around that by specifying a stubbed appeal: build_stubbed(:task, appeal: build_stubbed(:appeal) but that is more verbose than necessary. The default should be to easily allow stubbing factories along with their associations.