Improve GraphQL tests that use nil checks
What does this MR do and why?
This MR improves some of the GraphQL tests to make them less brittle.
Further details
I noticed that some of the GraphQL tests that use the form:
expect(subject.dig('data', '<some-field>')).to be_nil
Checking for a nil value this way can suffer from false negatives. For example, if we look at this test for Types::IssueType
:
let(:query) do
<<~GRAPHQL
query project($fullPath: ID!, $first: Int, $after: String) {
project(fullPath: $fullPath) {
issues(first: $first, after: $after) {
count
edges {
node {
iid
}
}
pageInfo {
endCursor
hasNextPage
}
}
}
}
GRAPHQL
end
context 'when user does not have the permission' do
it 'returns no data' do
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false)
expect(subject.dig(:data, :project)).to eq(nil)
end
end
This test will succeed even if we change the query to an invalid string:
let(:query) { "this is a test" }
context 'when user does not have the permission' do
it 'returns no data' do
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false)
expect(subject.dig(:data, :project)).to eq(nil)
end
end
Test environment set up in 12.195608 seconds
.
Finished in 19.03 seconds (files took 21.44 seconds to load)
1 example, 0 failures
However, if instead of using subject.dig(:data, :project)
we use array subscripts such as subject["data"]["project"]
, then we'll encounter a failure, as expected:
let(:query) { "this is a test" }
context 'when user does not have the permission' do
it 'returns no data' do
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false)
expect(subject["data"]["project"]).to eq(nil)
end
end
Failure/Error: expect(subject["data"]["project"]).to be_nil
NoMethodError:
undefined method `[]' for nil:NilClass
We can also add a check to ensure that subject["errors"]
are nil, which will provide a more descriptive failure than the above message:
it 'does not return an error' do
expect(subject['errors']).to be_nil
end
Failure/Error: expect(subject['errors']).to be_nil
expected: nil
got: [{"locations"=>[{"column"=>1, "line"=>1}], "message"=>"Parse error on \"this\" (IDENTIFIER) at [1, 1]"}]
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.