Repository: gphotosuploader/gphotos-uploader-cli
Pull request #154 (2019-11-10 11:46:23)

Add tests to album name generators
Signed-off-by: pacoorozco <paco@pacoorozco.info>

**What issue type does this pull request address?**
/kind enhancement


**What is this pull request for? Which issues does it resolve?** 
Add tests to album name generators. Some refactors has been done.


**Does this pull request has user-facing changes?** 
No


Patch:
diff --git a/cmd/push.go b/cmd/push.go
index 0e41112..0b4b3d4 100644
--- a/cmd/push.go
+++ b/cmd/push.go
@@ -96,8 +96,8 @@ func (cmd *PushCmd) Run(cobraCmd *cobra.Command, args []string) error {
 	}
 
 	// launch all folder upload jobs
-	for _, item := range cfg.Jobs {
-		c, err := app.NewOAuth2Client(ctx, oauth2Config, item.Account)
+	for _, config := range cfg.Jobs {
+		c, err := app.NewOAuth2Client(ctx, oauth2Config, config.Account)
 		if err != nil {
 			return err
 		}
@@ -107,23 +107,24 @@ func (cmd *PushCmd) Run(cobraCmd *cobra.Command, args []string) error {
 			return err
 		}
 
-		opt := upload.NewJobOptions(
-			item.MakeAlbums.Enabled,
-			item.MakeAlbums.Use,
-			item.DeleteAfterUpload,
-			upload.NewFilter(item.IncludePatterns, item.ExcludePatterns, item.UploadVideos),
-		)
-		job := upload.NewFolderUploadJob(gPhotos, app.FileTracker, item.SourceFolder, opt)
+		job := upload.NewFolderUploadJob(gPhotos, app.FileTracker, config.SourceFolder,
+			upload.JobOptions{
+				CreateAlbum:        config.MakeAlbums.Enabled,
+				CreateAlbumBasedOn: config.MakeAlbums.Use,
+				DeleteAfterUpload:  config.DeleteAfterUpload,
+				Filter:             upload.NewFilter(config.IncludePatterns, config.ExcludePatterns, config.UploadVideos),
+			})
 
-		// get items to be uploaded
-		jobs, err := job.ScanFolder(app.Logger)
+		// get Items{} to be uploaded to Google Photos.
+		items, err := job.ScanFolder(app.Logger)
 		if err != nil {
-			log.Fatalf("Failed to upload folder %s: %v", item.SourceFolder, err)
+			log.Fatalf("Failed to scan folder %s: %v", config.SourceFolder, err)
 		}
 
-		// enqueue items to be uploaded
-		for _, j := range jobs {
-			uploadChan <- &j
+		// enqueue Items{} to be uploaded. The workers will receive it via channel.
+		log.Infof("%d files pending to be uploaded in folder '%s'.", len(items), config.SourceFolder)
+		for _, i := range items {
+			uploadChan <- &i
 		}
 	}
 
diff --git a/upload/album.go b/upload/album.go
index c2277c9..0fd16f6 100644
--- a/upload/album.go
+++ b/upload/album.go
@@ -1,30 +1,32 @@
 package upload
 
 import (
-	"fmt"
 	"path/filepath"
 	"strings"
 
 	"github.com/gphotosuploader/gphotos-uploader-cli/log"
 )
 
-// albumID returns the album ID if MakeAlbums is enabled
+// albumID returns the album ID of the created (or existent) Album in Google Photos.
+// If configuration option `MakeAlbum.enabled` is false, it returns empty string.
 func (job *Job) albumID(path string, logger log.Logger) string {
-	if !job.options.createAlbum {
+	if !job.options.CreateAlbum {
 		return ""
 	}
 
-	name := albumNameUsingTemplate(path, job.options.createAlbumBasedOn)
+	// Album name can be customized using `MakeAlbums.use` configuration option.
+	name := albumNameUsingTemplate(path, job.options.CreateAlbumBasedOn)
 	if name == "" {
 		return ""
 	}
 
-	albumID, err := job.createAlbumInGPhotos(name)
+	// get or creates an Album with the calculated name.
+	album, err := job.gPhotos.GetOrCreateAlbumByName(name)
 	if err != nil {
-		logger.Error(err)
+		logger.Errorf("album creation failed: name=%s, error=%s", name, err)
 		return ""
 	}
-	return albumID
+	return album.Id
 }
 
 // albumNameUsingTemplate calculate the Album name for a given path based on full folder path (folderPath)
@@ -56,14 +58,3 @@ func albumNameUsingFolderName(path string) string {
 	}
 	return filepath.Base(p)
 }
-
-// createAlbumInGPhotos returns the ID of an album with the specified name or error if fails.
-// If the album didn't exist, it's created thanks to GetOrCreateAlbumByName().
-func (job *Job) createAlbumInGPhotos(name string) (string, error) {
-	// get album ID from Google Photos API
-	album, err := job.gPhotos.GetOrCreateAlbumByName(name)
-	if err != nil {
-		return "", fmt.Errorf("album creation failed: name=%s, error=%s", name, err)
-	}
-	return album.Id, nil
-}
diff --git a/upload/fileUpload.go b/upload/fileUpload.go
index 07b16c4..58312dd 100644
--- a/upload/fileUpload.go
+++ b/upload/fileUpload.go
@@ -3,8 +3,6 @@ package upload
 import (
 	"context"
 
-	gphotos "github.com/gphotosuploader/google-photos-api-client-go/lib-gphotos"
-
 	"github.com/gphotosuploader/gphotos-uploader-cli/app"
 	"github.com/gphotosuploader/gphotos-uploader-cli/log"
 )
@@ -12,15 +10,6 @@ import (
 // number of concurrent workers uploading items
 const maxNumberOfWorkers = 5
 
-// Item represents an object to be uploaded to Google Photos
-type Item struct {
-	gPhotos *gphotos.Client
-
-	path            string
-	album           string
-	deleteOnSuccess bool
-}
-
 // concurrentUpload read fileUploads chan for each Item struct, and upload the file to gphotos
 // when the fileUploadsChan is done, signal to doneUploading
 // TODO: We should refactor this to improve concurrency
diff --git a/upload/filter.go b/upload/filter.go
index 385b27f..1ec1191 100644
--- a/upload/filter.go
+++ b/upload/filter.go
@@ -6,7 +6,7 @@ import (
 	"github.com/gphotosuploader/gphotos-uploader-cli/match"
 )
 
-// Filter is a filter for file uploading
+// Filter is a Filter for file uploading
 type Filter struct {
 	isIncluded func(string) bool
 	isExcluded func(string) bool
diff --git a/upload/folderUploadJob.go b/upload/folderUploadJob.go
index 7d16212..0c28de9 100644
--- a/upload/folderUploadJob.go
+++ b/upload/folderUploadJob.go
@@ -4,56 +4,15 @@ import (
 	"os"
 	"path/filepath"
 
-	gphotos "github.com/gphotosuploader/google-photos-api-client-go/lib-gphotos"
-
-	"github.com/gphotosuploader/gphotos-uploader-cli/app"
 	"github.com/gphotosuploader/gphotos-uploader-cli/log"
 	"github.com/gphotosuploader/gphotos-uploader-cli/utils/filesystem"
 )
 
-// Job represents a job to upload all photos from the specified folder
-type Job struct {
-	gPhotos     *gphotos.Client
-	fileTracker app.FileTracker
-
-	sourceFolder string
-	options      *JobOptions
-}
-
-// JobOptions represents all the options that a job can have
-type JobOptions struct {
-	createAlbum        bool
-	createAlbumBasedOn string
-	deleteAfterUpload  bool
-	filter             *Filter
-}
-
-// NewJobOptions create a jobOptions based on the submitted / validated data
-func NewJobOptions(createAlbum bool, createAlbumBasedOn string, deleteAfterUpload bool, filter *Filter) *JobOptions {
-	return &JobOptions{
-		createAlbum:        createAlbum,
-		createAlbumBasedOn: createAlbumBasedOn,
-		deleteAfterUpload:  deleteAfterUpload,
-		filter:             filter,
-	}
-}
-
-// NewFolderUploadJob creates a job based on the submitted data
-func NewFolderUploadJob(gPhotos *gphotos.Client, fileTracker app.FileTracker, fp string, opt *JobOptions) *Job {
-	return &Job{
-		fileTracker: fileTracker,
-		gPhotos:     gPhotos,
-
-		sourceFolder: fp,
-		options:      opt,
-	}
-}
-
 // ScanFolder return the list of Items{} to be uploaded. It scans the folder and skip
 // non allowed files (includePatterns & excludePattens).
 func (job *Job) ScanFolder(logger log.Logger) ([]Item, error) {
 	var result []Item
-	err := filepath.Walk(job.sourceFolder, job.createUploadItemListFn(&result, job.options.filter, logger))
+	err := filepath.Walk(job.sourceFolder, job.createUploadItemListFn(&result, job.options.Filter, logger))
 	return result, err
 }
 
@@ -88,7 +47,7 @@ func (job *Job) createUploadItemListFn(items *[]Item, filter *Filter, logger log
 			gPhotos:         job.gPhotos,
 			path:            fp,
 			album:           job.albumID(relativePath, logger),
-			deleteOnSuccess: job.options.deleteAfterUpload,
+			deleteOnSuccess: job.options.DeleteAfterUpload,
 		})
 		return nil
 	}
diff --git a/upload/types.go b/upload/types.go
new file mode 100644
index 0000000..225a5d5
--- /dev/null
+++ b/upload/types.go
@@ -0,0 +1,52 @@
+package upload
+
+import (
+	"context"
+
+	"github.com/gphotosuploader/googlemirror/api/photoslibrary/v1"
+
+	"github.com/gphotosuploader/gphotos-uploader-cli/app"
+)
+
+// gPhotosService represents a Google Photos Service.
+type gPhotosService interface {
+	GetOrCreateAlbumByName(name string) (*photoslibrary.Album, error)
+	AddMediaItem(ctx context.Context, path string, album string) (*photoslibrary.MediaItem, error)
+}
+
+// Job represents a job to upload all photos from the specified folder
+type Job struct {
+	gPhotos     gPhotosService
+	fileTracker app.FileTracker
+
+	sourceFolder string
+	options      JobOptions
+}
+
+// NewFolderUploadJob creates a job based on the submitted data
+func NewFolderUploadJob(gPhotos gPhotosService, fileTracker app.FileTracker, sourceFolder string, opt JobOptions) *Job {
+	return &Job{
+		fileTracker: fileTracker,
+		gPhotos:     gPhotos,
+
+		sourceFolder: sourceFolder,
+		options:      opt,
+	}
+}
+
+// JobOptions represents all the options that a job can have
+type JobOptions struct {
+	CreateAlbum        bool
+	CreateAlbumBasedOn string
+	DeleteAfterUpload  bool
+	Filter             *Filter
+}
+
+// Item represents an object to be uploaded to Google Photos
+type Item struct {
+	gPhotos gPhotosService
+
+	path            string
+	album           string
+	deleteOnSuccess bool
+}


Test patch:
diff --git a/upload/album_test.go b/upload/album_test.go
index b396837..676f2be 100644
--- a/upload/album_test.go
+++ b/upload/album_test.go
@@ -1,9 +1,82 @@
 package upload
 
 import (
+	"context"
+	"errors"
 	"testing"
+
+	"github.com/gphotosuploader/googlemirror/api/photoslibrary/v1"
+
+	"github.com/gphotosuploader/gphotos-uploader-cli/log"
 )
 
+type MockGPhotosService struct {
+	AlbumID string
+}
+
+func (m *MockGPhotosService) GetOrCreateAlbumByName(name string) (*photoslibrary.Album, error) {
+	if name == "makeMyTestFail" {
+		return &photoslibrary.Album{}, errors.New("error")
+	}
+	return &photoslibrary.Album{Id: m.AlbumID}, nil
+}
+
+func (m *MockGPhotosService) AddMediaItem(ctx context.Context, path string, album string) (*photoslibrary.MediaItem, error) {
+	return &photoslibrary.MediaItem{}, nil
+}
+
+func TestAlbumId(t *testing.T) {
+	// set an AlbumID different than expected, in order to ensure it's returning empty
+	// due to CreateAlbum == false.
+	job := Job{
+		gPhotos: &MockGPhotosService{AlbumID: "testAlbumID"},
+		options: JobOptions{
+			CreateAlbum:        false,
+			CreateAlbumBasedOn: "folderPath",
+		},
+	}
+
+	t.Run("CreateAlbumDisabled", func(t *testing.T) {
+		job.options.CreateAlbum = false
+		want := ""
+		got := job.albumID("foo/bar/file.jpg", log.Discard)
+
+		if got != want {
+			t.Errorf("albumID test faild: expected '%s', got '%s'", want, got)
+		}
+	})
+
+	t.Run("WithEmptyPath", func(t *testing.T) {
+		job.options.CreateAlbum = true
+		want := ""
+		got := job.albumID("", log.Discard)
+
+		if got != want {
+			t.Errorf("albumID test faild: expected '%s', got '%s'", want, got)
+		}
+	})
+
+	t.Run("WithSuccessfulCall", func(t *testing.T) {
+		job.options.CreateAlbum = true
+		want := "testAlbumID"
+		got := job.albumID("foo/bar/file.jpg", log.Discard)
+
+		if got != want {
+			t.Errorf("albumID test faild: expected '%s', got '%s'", want, got)
+		}
+	})
+
+	t.Run("WithFailedCall", func(t *testing.T) {
+		job.options.CreateAlbum = true
+		want := ""
+		got := job.albumID("makeMyTestFail/file.jpg", log.Discard)
+
+		if got != want {
+			t.Errorf("albumID test faild: expected '%s', got '%s'", want, got)
+		}
+	})
+}
+
 func TestAlbumNameUsingTemplate(t *testing.T) {
 	var testData = []struct {
 		in       string
diff --git a/upload/filter_test.go b/upload/filter_test.go
index 1ec1a94..dd0914e 100644
--- a/upload/filter_test.go
+++ b/upload/filter_test.go
@@ -26,7 +26,7 @@ func TestFilter_AllowAllFiles(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -36,7 +36,7 @@ func TestFilter_AllowAllFiles(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -46,7 +46,7 @@ func TestFilter_AllowAllFiles(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -56,7 +56,7 @@ func TestFilter_AllowAllFiles(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -83,7 +83,7 @@ func TestFilter_AllowPNGFiles(t *testing.T) {
 	for _, tc := range testCases {
 		got := f.IsAllowed(tc.file)
 		if tc.out != got {
-			t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+			t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 		}
 	}
 
@@ -110,7 +110,7 @@ func TestFilter_AllowPNGAndJPGFiles(t *testing.T) {
 	for _, tc := range testCases {
 		got := f.IsAllowed(tc.file)
 		if tc.out != got {
-			t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+			t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 		}
 	}
 
@@ -135,7 +135,7 @@ func TestFilter_AllowImageFilesStartingWithSample(t *testing.T) {
 	for _, tc := range testCases {
 		got := f.IsAllowed(tc.file)
 		if tc.out != got {
-			t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+			t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 		}
 	}
 
@@ -161,7 +161,7 @@ func TestFilter_DisallowAllFiles(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -171,7 +171,7 @@ func TestFilter_DisallowAllFiles(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -197,7 +197,7 @@ func TestFilter_DisallowFilesStartingWithScreenShot(t *testing.T) {
 	for _, tc := range testCases {
 		got := f.IsAllowed(tc.file)
 		if tc.out != got {
-			t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+			t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 		}
 	}
 
@@ -223,7 +223,7 @@ func TestFilter_DisallowVideos(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -233,7 +233,7 @@ func TestFilter_DisallowVideos(t *testing.T) {
 		for _, tc := range testCases {
 			got := f.IsAllowed(tc.file)
 			if tc.out != got {
-				t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+				t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 			}
 		}
 	})
@@ -255,7 +255,7 @@ func TestFilter_DisallowAFolder(t *testing.T) {
 	for _, tc := range testCases {
 		got := f.IsAllowed(tc.file)
 		if tc.out != got {
-			t.Errorf("filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
+			t.Errorf("Filter result was not expected: file=%s, want %t, got %t", tc.file, tc.out, got)
 		}
 	}