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.