isolate: add uploadtracker tests for symlinks and isolated files

client/cmd/isolate/uploader.go

Index: client/cmd/isolate/uploader.go
diff --git a/client/cmd/isolate/uploader.go b/client/cmd/isolate/uploader.go
index a0f69624c203258521f867fafb35e2b4a9acc80f..0ce381eaddaef1353ceed2f94101e062c494876a 100644
--- a/client/cmd/isolate/uploader.go
+++ b/client/cmd/isolate/uploader.go
@@ -27,9 +27,18 @@ import (
 	"golang.org/x/net/context"
 )
 
-// Uploader uses an isolatedclient.Client to upload items to the server.
+// Uploader uploads items to the server.
+// It has a single implementation, *CappedUploader. See CappedUploader for method documentation.
+type Uploader interface {
+	Close() error
+	Upload(name string, src isolatedclient.Source, ps *isolatedclient.PushState, done func())
+	UploadBytes(name string, b []byte, ps *isolatedclient.PushState, done func())
+	UploadFile(item *Item, ps *isolatedclient.PushState, done func())
+}
+
+// CappedUploader uses an isolatedclient.Client to upload items to the server.

[mithro, 2017/07/31 02:34:26]

I'm not sure I understand why this is called a "CappedUploader"?

[mcgreevy, 2017/07/31 04:42:12]

I needed a name other than "Uploader", and I picked this because "Capped" does
describe on of its characteristics (it uses waitc to cap concurrent uploads).  I
supposed I could have chosen something generic like UploaderImpl, but that
sounds very C++-ey and in the end I decided that this name would not be very
visible anyway, so I decided to stop spending time on name-choosing.  Let me
know if you can think of something better.

 // All methods are safe for concurrent use.
-type Uploader struct {
+type CappedUploader struct {
 	ctx   context.Context
 	svc   isolateService
 	waitc chan bool // Used to cap concurrent uploads.
@@ -39,16 +48,16 @@ type Uploader struct {
 	err   error // The first error encountered, if any.
 }
 
-// NewUploader creates a new Uploader with the given isolated client.
+// NewUploader creates a new CappedUploader with the given isolated client.
 // maxConcurrent controls maximum number of uploads to be in-flight at once.
 // The provided context is used to make all requests to the isolate server.
-func NewUploader(ctx context.Context, client *isolatedclient.Client, maxConcurrent int) *Uploader {
+func NewUploader(ctx context.Context, client *isolatedclient.Client, maxConcurrent int) *CappedUploader {
 	return newUploader(ctx, client, maxConcurrent)
 }
 
-func newUploader(ctx context.Context, svc isolateService, maxConcurrent int) *Uploader {
+func newUploader(ctx context.Context, svc isolateService, maxConcurrent int) *CappedUploader {
 	const concurrentUploads = 10
-	return &Uploader{
+	return &CappedUploader{
 		ctx:   ctx,
 		svc:   svc,
 		waitc: make(chan bool, maxConcurrent),
@@ -58,7 +67,7 @@ func newUploader(ctx context.Context, svc isolateService, maxConcurrent int) *Up
 // Upload uploads an item from an isolated.Source. Upload does not block. If
 // not-nil, the done func will be invoked on upload completion (both success
 // and failure).
-func (u *Uploader) Upload(name string, src isolatedclient.Source, ps *isolatedclient.PushState, done func()) {
+func (u *CappedUploader) Upload(name string, src isolatedclient.Source, ps *isolatedclient.PushState, done func()) {
 	u.wg.Add(1)
 	go u.upload(name, src, ps, done)
 }
@@ -68,7 +77,7 @@ func (u *Uploader) Upload(name string, src isolatedclient.Source, ps *isolatedcl
 // and failure). The provided byte slice b must not be modified until the
 // upload is completed.
 // TODO(djd): Consider using Upload directly and deleting UploadBytes.
-func (u *Uploader) UploadBytes(name string, b []byte, ps *isolatedclient.PushState, done func()) {
+func (u *CappedUploader) UploadBytes(name string, b []byte, ps *isolatedclient.PushState, done func()) {
 	u.wg.Add(1)
 	go u.upload(name, byteSource(b), ps, done)
 }
@@ -77,7 +86,7 @@ func (u *Uploader) UploadBytes(name string, b []byte, ps *isolatedclient.PushSta
 // not-nil, the done func will be invoked on upload completion (both success
 // and failure).
 // TODO(djd): Consider using Upload directly and deleting UploadFile.
-func (u *Uploader) UploadFile(item *Item, ps *isolatedclient.PushState, done func()) {
+func (u *CappedUploader) UploadFile(item *Item, ps *isolatedclient.PushState, done func()) {
 	u.wg.Add(1)
 	go u.upload(item.RelPath, fileSource(item.Path), ps, done)
 }
@@ -85,13 +94,13 @@ func (u *Uploader) UploadFile(item *Item, ps *isolatedclient.PushState, done fun
 // Close waits for any pending uploads (and associated done callbacks) to
 // complete, and returns the first encountered error if any.
 // Uploader cannot be used once it is closed.
-func (u *Uploader) Close() error {
+func (u *CappedUploader) Close() error {
 	u.wg.Wait()
 	close(u.waitc) // Sanity check that we don't do any more uploading.
 	return u.err
 }
 
-func (u *Uploader) upload(name string, src isolatedclient.Source, ps *isolatedclient.PushState, done func()) {
+func (u *CappedUploader) upload(name string, src isolatedclient.Source, ps *isolatedclient.PushState, done func()) {
 	u.waitc <- true
 	defer func() {
 		<-u.waitc
@@ -113,13 +122,13 @@ func (u *Uploader) upload(name string, src isolatedclient.Source, ps *isolatedcl
 	}
 }
 
-func (u *Uploader) getErr() error {
+func (u *CappedUploader) getErr() error {
 	u.errMu.Lock()
 	defer u.errMu.Unlock()
 	return u.err
 }
 
-func (u *Uploader) setErr(err error) {
+func (u *CappedUploader) setErr(err error) {
 	u.errMu.Lock()
 	defer u.errMu.Unlock()
 	if u.err == nil {