@@ -145,7 +145,7 @@ func TestPushPublishedPackageRevision_PushDraftsDisabled(t *testing.T) {
145145
146146 tt .setupMocks (mockRepo , mockPR , mockPRD )
147147
148- _ , err := PushPackageRevision (ctx , mockRepo , mockPR , false , nil )
148+ _ , _ , err := PushPublishedPackageRevision (ctx , mockRepo , mockPR , false , false )
149149 if tt .expectError {
150150 assert .NotNil (t , err )
151151 } else {
@@ -159,71 +159,63 @@ func TestPushPublishedPackageRevision_PushDraftsEnabled(t *testing.T) {
159159 ctx := context .TODO ()
160160
161161 tests := []struct {
162- name string
163- setupMocks func (* mockrepo.MockRepository , * mockrepo.MockPackageRevision , * mockrepo.MockPackageRevision , * mockrepo.MockPackageRevisionDraft )
164- gitPR bool
165- expectError bool
162+ name string
163+ setupMocks func (* mockrepo.MockRepository , * mockrepo.MockPackageRevision , * mockrepo.MockPackageRevision , * mockrepo.MockPackageRevisionDraft )
164+ existingGitBranch bool
165+ expectError bool
166166 }{
167167 {
168- name : "Update existing PR" ,
169- gitPR : true ,
170- setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
171- mockPR .EXPECT ().Lifecycle (mock .Anything ).Return (porchapi .PackageRevisionLifecyclePublished ).Once ()
172- mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
173- mockPR .EXPECT ().GetResources (mock .Anything ).Return (& porchapi.PackageRevisionResources {}, nil ).Once ()
174- mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (mockPRD , nil ).Once ()
175- mockPRD .EXPECT ().UpdateLifecycle (mock .Anything , porchapi .PackageRevisionLifecyclePublished ).Return (nil ).Once ()
176- mockRepo .EXPECT ().ClosePackageRevisionDraft (mock .Anything , mockPRD , mock .Anything ).Return (mockPR , nil ).Once ()
177- mockPR .EXPECT ().GetLock (mock .Anything ).Return (kptfilev1.Upstream {}, kptfilev1.Locator {}, nil ).Once ()
178- },
179- expectError : false ,
180- },
181- {
182- name : "Existing PR found via list" ,
183- gitPR : false ,
168+ name : "Update existing PR" ,
169+ existingGitBranch : true ,
184170 setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
185171 mockPR .EXPECT ().Lifecycle (mock .Anything ).Return (porchapi .PackageRevisionLifecyclePublished ).Once ()
186172 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
187173 mockPR .EXPECT ().GetResources (mock .Anything ).Return (& porchapi.PackageRevisionResources {}, nil ).Once ()
188174 mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {mockGitPR }, nil ).Once ()
189175 mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (mockPRD , nil ).Once ()
176+ mockPRD .EXPECT ().UpdateResources (mock .Anything , mock .Anything , mock .Anything ).Return (nil ).Once ()
190177 mockPRD .EXPECT ().UpdateLifecycle (mock .Anything , porchapi .PackageRevisionLifecyclePublished ).Return (nil ).Once ()
191178 mockRepo .EXPECT ().ClosePackageRevisionDraft (mock .Anything , mockPRD , mock .Anything ).Return (mockPR , nil ).Once ()
192179 mockPR .EXPECT ().GetLock (mock .Anything ).Return (kptfilev1.Upstream {}, kptfilev1.Locator {}, nil ).Once ()
193180 },
194181 expectError : false ,
195182 },
196183 {
197- name : "UpdatePackageRevision fails when gitPR provided " ,
198- gitPR : true ,
184+ name : "UpdatePackageRevision fails when existing git branch found " ,
185+ existingGitBranch : true ,
199186 setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
200187 mockPR .EXPECT ().Lifecycle (mock .Anything ).Return (porchapi .PackageRevisionLifecyclePublished ).Once ()
201188 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
202189 mockPR .EXPECT ().GetResources (mock .Anything ).Return (& porchapi.PackageRevisionResources {}, nil ).Once ()
190+ mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {mockGitPR }, nil ).Once ()
203191 mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (nil , assert .AnError ).Once ()
204192 },
205193 expectError : true ,
206194 },
207195 {
208- name : "UpdatePackageRevision fails when gitPR found via list " ,
209- gitPR : false ,
196+ name : "ListPackageRevisions fails and falls back to creating package revision draft " ,
197+ existingGitBranch : true ,
210198 setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
211199 mockPR .EXPECT ().Lifecycle (mock .Anything ).Return (porchapi .PackageRevisionLifecyclePublished ).Once ()
212- mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
200+ mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {Spec : porchapi. PackageRevisionSpec { Tasks : []porchapi. Task {{ Type : porchapi . TaskTypePush }}} }, nil ).Once ()
213201 mockPR .EXPECT ().GetResources (mock .Anything ).Return (& porchapi.PackageRevisionResources {}, nil ).Once ()
214- mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {mockGitPR }, nil ).Once ()
215- mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (nil , assert .AnError ).Once ()
202+ mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return (nil , assert .AnError ).Once ()
203+ mockRepo .EXPECT ().CreatePackageRevisionDraft (mock .Anything , mock .Anything ).Return (mockPRD , nil ).Once ()
204+ mockPRD .EXPECT ().UpdateResources (mock .Anything , mock .Anything , mock .Anything ).Return (nil ).Once ()
205+ mockPRD .EXPECT ().UpdateLifecycle (mock .Anything , porchapi .PackageRevisionLifecyclePublished ).Return (nil ).Once ()
206+ mockRepo .EXPECT ().ClosePackageRevisionDraft (mock .Anything , mockPRD , mock .Anything ).Return (mockPR , nil ).Once ()
207+ mockPR .EXPECT ().GetLock (mock .Anything ).Return (kptfilev1.Upstream {}, kptfilev1.Locator {}, nil ).Once ()
216208 },
217- expectError : true ,
209+ expectError : false ,
218210 },
219211 {
220- name : "ListPackageRevisions fails and falls back to creating package revision draft" ,
221- gitPR : false ,
212+ name : "ListPackageRevisions returns empty and falls back to creating package revision draft" ,
213+ existingGitBranch : true ,
222214 setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
223215 mockPR .EXPECT ().Lifecycle (mock .Anything ).Return (porchapi .PackageRevisionLifecyclePublished ).Once ()
224- mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {Spec : porchapi. PackageRevisionSpec { Tasks : []porchapi. Task {{ Type : porchapi . TaskTypePush }}} }, nil ).Once ()
216+ mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
225217 mockPR .EXPECT ().GetResources (mock .Anything ).Return (& porchapi.PackageRevisionResources {}, nil ).Once ()
226- mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return (nil , assert . AnError ).Once ()
218+ mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository. PackageRevision {}, nil ).Once ()
227219 mockRepo .EXPECT ().CreatePackageRevisionDraft (mock .Anything , mock .Anything ).Return (mockPRD , nil ).Once ()
228220 mockPRD .EXPECT ().UpdateResources (mock .Anything , mock .Anything , mock .Anything ).Return (nil ).Once ()
229221 mockPRD .EXPECT ().UpdateLifecycle (mock .Anything , porchapi .PackageRevisionLifecyclePublished ).Return (nil ).Once ()
@@ -233,13 +225,12 @@ func TestPushPublishedPackageRevision_PushDraftsEnabled(t *testing.T) {
233225 expectError : false ,
234226 },
235227 {
236- name : "ListPackageRevisions returns empty and falls back to creating package revision draft" ,
237- gitPR : false ,
228+ name : "Creates new draft when no existing git branch " ,
229+ existingGitBranch : false ,
238230 setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
239231 mockPR .EXPECT ().Lifecycle (mock .Anything ).Return (porchapi .PackageRevisionLifecyclePublished ).Once ()
240232 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
241233 mockPR .EXPECT ().GetResources (mock .Anything ).Return (& porchapi.PackageRevisionResources {}, nil ).Once ()
242- mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {}, nil ).Once ()
243234 mockRepo .EXPECT ().CreatePackageRevisionDraft (mock .Anything , mock .Anything ).Return (mockPRD , nil ).Once ()
244235 mockPRD .EXPECT ().UpdateResources (mock .Anything , mock .Anything , mock .Anything ).Return (nil ).Once ()
245236 mockPRD .EXPECT ().UpdateLifecycle (mock .Anything , porchapi .PackageRevisionLifecyclePublished ).Return (nil ).Once ()
@@ -260,14 +251,9 @@ func TestPushPublishedPackageRevision_PushDraftsEnabled(t *testing.T) {
260251 mockRepo .EXPECT ().Key ().Return (repository.RepositoryKey {}).Maybe ()
261252 mockPR .EXPECT ().Key ().Return (repository.PackageRevisionKey {}).Maybe ()
262253
263- var gitPR repository.PackageRevision
264- if tt .gitPR {
265- gitPR = mockGitPR
266- }
267-
268254 tt .setupMocks (mockRepo , mockPR , mockGitPR , mockPRD )
269255
270- _ , err := PushPackageRevision (ctx , mockRepo , mockPR , true , gitPR )
256+ _ , _ , err := PushPublishedPackageRevision (ctx , mockRepo , mockPR , true , tt . existingGitBranch )
271257 if tt .expectError {
272258 assert .NotNil (t , err )
273259 } else {
@@ -283,35 +269,15 @@ func TestGetOrCreateGitDraft(t *testing.T) {
283269 tests := []struct {
284270 name string
285271 setupMocks func (* mockrepo.MockRepository , * mockrepo.MockPackageRevision , * mockrepo.MockPackageRevision , * mockrepo.MockPackageRevisionDraft )
286- gitPR bool
287272 expectError bool
288273 expectUpdatedGitPR bool
289274 }{
290275 {
291- name : "UpdatePackageRevision succeeds" ,
292- setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
293- mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (mockPRD , nil ).Once ()
294- },
295- gitPR : true ,
296- expectError : false ,
297- expectUpdatedGitPR : true ,
298- },
299- {
300- name : "UpdatePackageRevision fails" ,
301- setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
302- mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (nil , assert .AnError ).Once ()
303- },
304- gitPR : true ,
305- expectError : true ,
306- expectUpdatedGitPR : false ,
307- },
308- {
309- name : "Existing PRs found" ,
276+ name : "UpdatePackageRevision succeeds when existing PRs found" ,
310277 setupMocks : func (mockRepo * mockrepo.MockRepository , mockPR * mockrepo.MockPackageRevision , mockGitPR * mockrepo.MockPackageRevision , mockPRD * mockrepo.MockPackageRevisionDraft ) {
311278 mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {mockGitPR }, nil ).Once ()
312279 mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (mockPRD , nil ).Once ()
313280 },
314- gitPR : false ,
315281 expectError : false ,
316282 expectUpdatedGitPR : true ,
317283 },
@@ -321,7 +287,6 @@ func TestGetOrCreateGitDraft(t *testing.T) {
321287 mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {mockGitPR }, nil ).Once ()
322288 mockRepo .EXPECT ().UpdatePackageRevision (mock .Anything , mockGitPR ).Return (nil , assert .AnError ).Once ()
323289 },
324- gitPR : false ,
325290 expectError : true ,
326291 expectUpdatedGitPR : false ,
327292 },
@@ -332,7 +297,6 @@ func TestGetOrCreateGitDraft(t *testing.T) {
332297 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
333298 mockRepo .EXPECT ().CreatePackageRevisionDraft (mock .Anything , mock .Anything ).Return (mockPRD , nil ).Once ()
334299 },
335- gitPR : false ,
336300 expectError : false ,
337301 expectUpdatedGitPR : false ,
338302 },
@@ -343,7 +307,6 @@ func TestGetOrCreateGitDraft(t *testing.T) {
343307 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
344308 mockRepo .EXPECT ().CreatePackageRevisionDraft (mock .Anything , mock .Anything ).Return (mockPRD , nil ).Once ()
345309 },
346- gitPR : false ,
347310 expectError : false ,
348311 expectUpdatedGitPR : false ,
349312 },
@@ -353,7 +316,6 @@ func TestGetOrCreateGitDraft(t *testing.T) {
353316 mockRepo .EXPECT ().ListPackageRevisions (mock .Anything , mock .Anything ).Return ([]repository.PackageRevision {}, nil ).Once ()
354317 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (nil , assert .AnError ).Once ()
355318 },
356- gitPR : false ,
357319 expectError : true ,
358320 expectUpdatedGitPR : false ,
359321 },
@@ -364,7 +326,6 @@ func TestGetOrCreateGitDraft(t *testing.T) {
364326 mockPR .EXPECT ().GetPackageRevision (mock .Anything ).Return (& porchapi.PackageRevision {}, nil ).Once ()
365327 mockRepo .EXPECT ().CreatePackageRevisionDraft (mock .Anything , mock .Anything ).Return (nil , assert .AnError ).Once ()
366328 },
367- gitPR : false ,
368329 expectError : true ,
369330 expectUpdatedGitPR : false ,
370331 },
@@ -379,14 +340,9 @@ func TestGetOrCreateGitDraft(t *testing.T) {
379340
380341 mockPR .EXPECT ().Key ().Return (repository.PackageRevisionKey {}).Maybe ()
381342
382- var gitPR repository.PackageRevision
383- if tt .gitPR {
384- gitPR = mockGitPR
385- }
386-
387343 tt .setupMocks (mockRepo , mockPR , mockGitPR , mockPRD )
388344
389- draft , updatedGitPR , err := GetOrCreateGitDraft (ctx , mockRepo , mockPR , gitPR )
345+ draft , updatedGitPR , err := GetOrCreateGitDraft (ctx , mockRepo , mockPR )
390346
391347 if tt .expectError {
392348 assert .NotNil (t , err )
0 commit comments