RSpec shared example `412 response` naming is non-descriptive
This shared example for the REST API is named poorly. We should be describing what this covers better. I don't think many devs even understand what this covers looking at the actual logic.
def destroy_conditionally!(resource, last_updated: nil)
last_updated ||= resource.updated_at
check_unmodified_since!(last_updated)
status 204
body false
if block_given?
yield resource
else
resource.destroy
end
end
We have this API helper method to conditionally destroy a resource which ensure that the record hasn't been modified since we fetched it from the database (or a specified value in the kwargs)
I think this method naming isn't great either but that's another issue.
The shared examples should be named something like "it can be destroyed conditionally". Currently we have specs that duplicate this test because the naming is confusing e.g. https://gitlab.com/gitlab-org/gitlab/-/blob/9082b8ce7e94449b73acd7e93c64501a86e94a3d/spec/requests/api/protected_tags_spec.rb#L216-224
it 'unprotects a single Protected Tag' do
delete api("/projects/#{project.id}/protected_tags/#{tag_name}", user)
expect(response).to have_gitlab_http_status(:no_content)
end
it_behaves_like '412 response' do
let(:request) { api("/projects/#{project.id}/protected_tags/#{tag_name}", user) }
end
If we refactor this to remove the it 'unprotects a single Protected Tag' spec it looks like we are testing a success flow.