Follow-up from "Add the Play button for delayed jobs in environment page"
The following discussion from !22106 (merged) should be addressed:
-
@nolith started a discussion: I don't think these 3 tests are good ones.
They are missing the point of checking to which method we are delegating.
A change like the following must have failed the test on line 14, but it didn't 'cause we are only checking that this is calling
try
.def manual_actions - @manual_actions ||= deployable.try(:other_actions) + @manual_actions ||= deployable.try(:other_manual_actions) end
I think we should replace those tests with something like
describe '#commit_title' do let(:project) { create(:project, :repository) } let(:environment) { create(:environment, project: project) } subject { create(:deployment, environment: environment, sha: project.commit.id) } it 'delegates to commit.title' do expect_any_instance_of(Commit).to receive(:try).with(:title).and_call_original subject.commit_title end end describe '#manual_actions' do let(:project) { create(:project, :repository) } let(:pipeline) { create(:ci_pipeline, project: project) } let(:build) { create(:ci_build, :success, pipeline: pipeline) } subject { create(:deployment, deployable: build) } it 'delegates to deployable.other_manual_actions' do expect(build).to receive(:try).with(:other_manaul_actions).and_call_original subject.manual_actions end end describe '#scheduled_actions' #bla bla bla
you can leave
it { is_expected.to delegate_method(:commit_title).to(:commit).as(:try) }
as is in order to keep this MR scoped to what we are doing. And create a follow-up issue for that fix.