fix upload retry logic

[#126926555]

out_command.go

@@ -130,40 +130,10 @@ func (c *OutCommand) Run(sourceDir string, request OutRequest) (OutResponse, err
 		}
 
 		for _, filePath := range matches {
-			file, err := os.Open(filePath)
+			err := c.upload(release, filePath)
 			if err != nil {
 				return OutResponse{}, err
 			}
-
-			fmt.Fprintf(c.writer, "uploading %s\n", filePath)
-
-			name := filepath.Base(filePath)
-
-			err = c.github.UploadReleaseAsset(*release, name, file)
-			for i := 0; i < 9 && err != nil; i++ {
-				assets, err := c.github.ListReleaseAssets(*release)
-				if err != nil {
-					return OutResponse{}, err
-				}
-
-				for _, asset := range assets {
-					if asset.Name != nil && *asset.Name == name {
-						err = c.github.DeleteReleaseAsset(*asset)
-						if err != nil {
-							return OutResponse{}, err
-						}
-						break
-					}
-				}
-
-				err = c.github.UploadReleaseAsset(*release, name, file)
-			}
-
-			if err != nil {
-				return OutResponse{}, err
-			}
-
-			file.Close()
 		}
 	}
 
@@ -181,3 +151,42 @@ func (c *OutCommand) fileContents(path string) (string, error) {
 
 	return strings.TrimSpace(string(contents)), nil
 }
+
+func (c *OutCommand) upload(release *github.RepositoryRelease, filePath string) error {
+	file, err := os.Open(filePath)
+	if err != nil {
+		return err
+	}
+
+	defer file.Close()
+
+	fmt.Fprintf(c.writer, "uploading %s\n", filePath)
+
+	name := filepath.Base(filePath)
+
+	retryErr := c.github.UploadReleaseAsset(*release, name, file)
+	for i := 0; i < 9 && retryErr != nil; i++ {
+		assets, err := c.github.ListReleaseAssets(*release)
+		if err != nil {
+			return err
+		}
+
+		for _, asset := range assets {
+			if asset.Name != nil && *asset.Name == name {
+				err = c.github.DeleteReleaseAsset(*asset)
+				if err != nil {
+					return err
+				}
+				break
+			}
+		}
+
+		retryErr = c.github.UploadReleaseAsset(*release, name, file)
+	}
+
+	if retryErr != nil {
+		return retryErr
+	}
+
+	return nil
+}

out_command_test.go

@@ -441,12 +441,46 @@ var _ = Describe("Out Command", func() {
 					actualRelease, actualName, actualFile := githubClient.UploadReleaseAssetArgsForCall(9)
 					Ω(*actualRelease.ID).Should(Equal(112))
 					Ω(actualName).Should(Equal("great-file.tgz"))
-					Ω(ioutil.ReadAll(actualFile)).Should(Equal([]byte("matching")))
+					Ω(actualFile.Name()).Should(Equal(filepath.Join(sourcesDir, "great-file.tgz")))
 
 					Ω(githubClient.DeleteReleaseAssetCallCount()).Should(Equal(9))
 					actualAsset := githubClient.DeleteReleaseAssetArgsForCall(8)
 					Expect(*actualAsset.ID).To(Equal(456789))
 				})
+
+				Context("when uploading succeeds on the 5th attempt", func() {
+					BeforeEach(func() {
+						results := make(chan error, 6)
+						results <- errors.New("1")
+						results <- errors.New("2")
+						results <- errors.New("3")
+						results <- errors.New("4")
+						results <- nil
+						results <- errors.New("6")
+
+						githubClient.UploadReleaseAssetStub = func(github.RepositoryRelease, string, *os.File) error {
+							return <-results
+						}
+					})
+
+					It("succeeds", func() {
+						_, err := command.Run(sourcesDir, request)
+						Expect(err).ToNot(HaveOccurred())
+
+						Ω(githubClient.UploadReleaseAssetCallCount()).Should(Equal(5))
+						Ω(githubClient.ListReleaseAssetsCallCount()).Should(Equal(4))
+						Ω(*githubClient.ListReleaseAssetsArgsForCall(3).ID).Should(Equal(112))
+
+						actualRelease, actualName, actualFile := githubClient.UploadReleaseAssetArgsForCall(4)
+						Ω(*actualRelease.ID).Should(Equal(112))
+						Ω(actualName).Should(Equal("great-file.tgz"))
+						Ω(actualFile.Name()).Should(Equal(filepath.Join(sourcesDir, "great-file.tgz")))
+
+						Ω(githubClient.DeleteReleaseAssetCallCount()).Should(Equal(4))
+						actualAsset := githubClient.DeleteReleaseAssetArgsForCall(3)
+						Expect(*actualAsset.ID).To(Equal(456789))
+					})
+				})
 			})
 		})