From 32f721320b4d2104a23d79dd15fa08ff06142944 Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 22 Mar 2022 13:45:35 +0800 Subject: [PATCH 01/10] Delete the zip files. (#347) --- go.sum | 1 - provisioner.go | 12 ++++--- test/file_test.go | 82 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 test/file_test.go diff --git a/go.sum b/go.sum index 562ea2f..8bdd880 100644 --- a/go.sum +++ b/go.sum @@ -4,7 +4,6 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/provisioner.go b/provisioner.go index 8eee913..cef0f1a 100644 --- a/provisioner.go +++ b/provisioner.go @@ -146,16 +146,18 @@ func (p *defaultProvisioner) updateProvisionStatus(paths Paths, s *ProvisionStat // provisionAstilectron provisions astilectron func (p *defaultProvisioner) provisionAstilectron(ctx context.Context, paths Paths, s ProvisionStatus, versionAstilectron string) error { - return p.provisionPackage(ctx, paths, s.Astilectron, p.moverAstilectron, "Astilectron", versionAstilectron, paths.AstilectronUnzipSrc(), paths.AstilectronDirectory(), nil) + return p.provisionPackage(ctx, paths, s.Astilectron, p.moverAstilectron, "Astilectron", versionAstilectron, paths.AstilectronUnzipSrc(), paths.AstilectronDirectory(), func() error { + return os.Remove(paths.astilectronDownloadDst) + }) } // provisionElectron provisions electron -func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, s ProvisionStatus, appName, os, arch, versionElectron string) error { +func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, s ProvisionStatus, appName, sysOS, arch, versionElectron string) error { if paths.ElectronUnzipSrc() == "" { return nil } - return p.provisionPackage(ctx, paths, s.Electron[provisionStatusElectronKey(os, arch)], p.moverElectron, "Electron", versionElectron, paths.ElectronUnzipSrc(), paths.ElectronDirectory(), func() (err error) { - switch os { + return p.provisionPackage(ctx, paths, s.Electron[provisionStatusElectronKey(sysOS, arch)], p.moverElectron, "Electron", versionElectron, paths.ElectronUnzipSrc(), paths.ElectronDirectory(), func() (err error) { + switch sysOS { case "darwin": if err = p.provisionElectronFinishDarwin(appName, paths); err != nil { return fmt.Errorf("finishing provisioning electron for darwin systems failed: %w", err) @@ -163,7 +165,7 @@ func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, default: p.l.Debug("System doesn't require finshing provisioning electron, moving on...") } - return + return os.Remove(paths.electronDownloadDst) }) } diff --git a/test/file_test.go b/test/file_test.go new file mode 100644 index 0000000..fe824db --- /dev/null +++ b/test/file_test.go @@ -0,0 +1,82 @@ +package main_test + +import ( + "fmt" + "github.com/asticode/go-astilectron" + "log" + "os" + "path/filepath" + "runtime" + "testing" + "time" +) + +func TestZipShouldRemove(t *testing.T) { + l := log.New(log.Writer(), log.Prefix(), log.Flags()) + + appName := "Test" + dataDir, _ := filepath.Abs("./temp/Test") + vendorDir := filepath.Join(dataDir, "vendor") // https://github.com/CarsonSlovoka/go-astilectron/blob/e7796e5/paths.go#L56 + astilectronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("astilectron-v%s.zip", astilectron.DefaultVersionAstilectron)) + astilectronDir := filepath.Join(vendorDir, fmt.Sprintf("astilectron")) + electronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s-v%s.zip", runtime.GOOS, runtime.GOARCH, astilectron.DefaultVersionElectron)) + electronDir := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s", runtime.GOOS, runtime.GOARCH)) + + t.Log("Make sure the test directory doesn't exist.") + if err := os.RemoveAll(dataDir); err != nil && !os.IsNotExist(err) { + t.FailNow() + } + + t.Logf("Create a directory for test only:\n%s\n", dataDir) + if err := os.MkdirAll(dataDir, os.FileMode(0666)); err != nil { + t.Fatalf(err.Error()) + } + + { + t.Log("common process") + a, err := astilectron.New(l, astilectron.Options{ + AppName: appName, + DataDirectoryPath: dataDir, + }) + if err != nil { + t.Fatalf("main: creating astilectron failed: %s", err.Error()) + } + defer a.Close() + + a.HandleSignals() + + if err = a.Start(); err != nil { + t.Fatalf("main: starting astilectron failed: %s", err.Error()) + } + + // Close the app immediately since we care about the file only to avoid access being denied. (delete) + a.Close() + time.Sleep(10 * time.Second) + t.Log("astilectron Close") + } + + { + t.Log("Check UnZip successful") + if _, err := os.Stat(astilectronDir); os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + + if _, err := os.Stat(electronDir); os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + + t.Log("Check Zip doesn't exist") + if _, err := os.Stat(astilectronDownloadDst); !os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + + if _, err := os.Stat(electronDownloadDst); !os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + } + + t.Log("Remove the test directory after the test was done.") + if err := os.RemoveAll(dataDir); err != nil { + t.Fatalf(err.Error()) + } +} From bc684fd5de747fc352bf607b3bbd70251a18dc48 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 25 Mar 2022 15:00:57 +0800 Subject: [PATCH 02/10] change the mover interface to func(context.Context, Paths) (func() error, error) delete the zip on the mover. https://github.com/asticode/go-astilectron/pull/358#issuecomment-1077386154 --- provisioner.go | 70 +++++++++++++++++++++++++++++++++-------------- test/file_test.go | 16 ++++++----- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/provisioner.go b/provisioner.go index cef0f1a..802d431 100644 --- a/provisioner.go +++ b/provisioner.go @@ -23,7 +23,7 @@ type Provisioner interface { } // mover is a function that moves a package -type mover func(ctx context.Context, p Paths) error +type mover func(ctx context.Context, p Paths) (func() error, error) // defaultProvisioner represents the default provisioner type defaultProvisioner struct { @@ -39,17 +39,27 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { }, }) dp = &defaultProvisioner{l: astikit.AdaptStdLogger(l)} - dp.moverAstilectron = func(ctx context.Context, p Paths) (err error) { + dp.moverAstilectron = func(ctx context.Context, p Paths) (closeFunc func() error, err error) { if err = Download(ctx, dp.l, d, p.AstilectronDownloadSrc(), p.AstilectronDownloadDst()); err != nil { - return fmt.Errorf("downloading %s into %s failed: %w", p.AstilectronDownloadSrc(), p.AstilectronDownloadDst(), err) + return nil, fmt.Errorf("downloading %s into %s failed: %w", p.AstilectronDownloadSrc(), p.AstilectronDownloadDst(), err) } - return + return func() (err error) { + if err = os.Remove(p.AstilectronDownloadDst()); err != nil { + err = fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) + } + return + }, err } - dp.moverElectron = func(ctx context.Context, p Paths) (err error) { + dp.moverElectron = func(ctx context.Context, p Paths) (closeFunc func() error, err error) { if err = Download(ctx, dp.l, d, p.ElectronDownloadSrc(), p.ElectronDownloadDst()); err != nil { - return fmt.Errorf("downloading %s into %s failed: %w", p.ElectronDownloadSrc(), p.ElectronDownloadDst(), err) + return nil, fmt.Errorf("downloading %s into %s failed: %w", p.ElectronDownloadSrc(), p.ElectronDownloadDst(), err) } - return + return func() (err error) { + if err = os.Remove(p.ElectronDownloadDst()); err != nil { + err = fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) + } + return + }, err } return } @@ -146,18 +156,16 @@ func (p *defaultProvisioner) updateProvisionStatus(paths Paths, s *ProvisionStat // provisionAstilectron provisions astilectron func (p *defaultProvisioner) provisionAstilectron(ctx context.Context, paths Paths, s ProvisionStatus, versionAstilectron string) error { - return p.provisionPackage(ctx, paths, s.Astilectron, p.moverAstilectron, "Astilectron", versionAstilectron, paths.AstilectronUnzipSrc(), paths.AstilectronDirectory(), func() error { - return os.Remove(paths.astilectronDownloadDst) - }) + return p.provisionPackage(ctx, paths, s.Astilectron, p.moverAstilectron, "Astilectron", versionAstilectron, paths.AstilectronUnzipSrc(), paths.AstilectronDirectory(), nil) } // provisionElectron provisions electron -func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, s ProvisionStatus, appName, sysOS, arch, versionElectron string) error { +func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, s ProvisionStatus, appName, os, arch, versionElectron string) error { if paths.ElectronUnzipSrc() == "" { return nil } - return p.provisionPackage(ctx, paths, s.Electron[provisionStatusElectronKey(sysOS, arch)], p.moverElectron, "Electron", versionElectron, paths.ElectronUnzipSrc(), paths.ElectronDirectory(), func() (err error) { - switch sysOS { + return p.provisionPackage(ctx, paths, s.Electron[provisionStatusElectronKey(os, arch)], p.moverElectron, "Electron", versionElectron, paths.ElectronUnzipSrc(), paths.ElectronDirectory(), func() (err error) { + switch os { case "darwin": if err = p.provisionElectronFinishDarwin(appName, paths); err != nil { return fmt.Errorf("finishing provisioning electron for darwin systems failed: %w", err) @@ -165,7 +173,7 @@ func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, default: p.l.Debug("System doesn't require finshing provisioning electron, moving on...") } - return os.Remove(paths.electronDownloadDst) + return }) } @@ -185,10 +193,20 @@ func (p *defaultProvisioner) provisionPackage(ctx context.Context, paths Paths, } // Move - if err = m(ctx, paths); err != nil { + var closeFunc func() error + if closeFunc, err = m(ctx, paths); err != nil { return fmt.Errorf("moving %s failed: %w", name, err) } + // Make sure to close + defer func() { + if err := closeFunc(); err != nil { + // Only log the error + p.l.Error(fmt.Errorf("closing failed: %w", err)) + return + } + }() + // Create directory p.l.Debugf("Creating directory %s", pathDirectory) if err = os.MkdirAll(pathDirectory, 0755); err != nil { @@ -332,17 +350,27 @@ type Disembedder func(src string) ([]byte, error) // NewDisembedderProvisioner creates a provisioner that can provision based on embedded data func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron string, l astikit.StdLogger) Provisioner { dp := &defaultProvisioner{l: astikit.AdaptStdLogger(l)} - dp.moverAstilectron = func(ctx context.Context, p Paths) (err error) { + dp.moverAstilectron = func(ctx context.Context, p Paths) (closeFunc func() error, err error) { if err = Disembed(ctx, dp.l, d, pathAstilectron, p.AstilectronDownloadDst()); err != nil { - return fmt.Errorf("disembedding %s into %s failed: %w", pathAstilectron, p.AstilectronDownloadDst(), err) + return nil, fmt.Errorf("disembedding %s into %s failed: %w", pathAstilectron, p.AstilectronDownloadDst(), err) } - return + return func() (err error) { + if err = os.Remove(p.AstilectronDownloadDst()); err != nil { + err = fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) + } + return + }, err } - dp.moverElectron = func(ctx context.Context, p Paths) (err error) { + dp.moverElectron = func(ctx context.Context, p Paths) (closeFunc func() error, err error) { if err = Disembed(ctx, dp.l, d, pathElectron, p.ElectronDownloadDst()); err != nil { - return fmt.Errorf("disembedding %s into %s failed: %w", pathElectron, p.ElectronDownloadDst(), err) + return nil, fmt.Errorf("disembedding %s into %s failed: %w", pathElectron, p.ElectronDownloadDst(), err) } - return + return func() (err error) { + if err = os.Remove(p.ElectronDownloadDst()); err != nil { + err = fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) + } + return + }, err } return dp } diff --git a/test/file_test.go b/test/file_test.go index fe824db..1f63e4f 100644 --- a/test/file_test.go +++ b/test/file_test.go @@ -32,6 +32,13 @@ func TestZipShouldRemove(t *testing.T) { t.Fatalf(err.Error()) } + defer func() { + t.Log("Remove the test directory after the test was done.") + if err := os.RemoveAll(dataDir); err != nil { + t.Fatalf(err.Error()) + } + }() + { t.Log("common process") a, err := astilectron.New(l, astilectron.Options{ @@ -49,8 +56,8 @@ func TestZipShouldRemove(t *testing.T) { t.Fatalf("main: starting astilectron failed: %s", err.Error()) } - // Close the app immediately since we care about the file only to avoid access being denied. (delete) - a.Close() + // Stop the app immediately since we care about the file only to avoid access being denied. (delete) + a.Stop() time.Sleep(10 * time.Second) t.Log("astilectron Close") } @@ -74,9 +81,4 @@ func TestZipShouldRemove(t *testing.T) { t.Fatalf(err.Error()) } } - - t.Log("Remove the test directory after the test was done.") - if err := os.RemoveAll(dataDir); err != nil { - t.Fatalf(err.Error()) - } } From e0552444a0d91cbae36e8c9a029b69a721179929 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 25 Mar 2022 15:13:51 +0800 Subject: [PATCH 03/10] move TestZipShouldRemove to provisioner_test.go put the test code together. --- provisioner_test.go | 76 ++++++++++++++++++++++++++++++++++++++++ test/file_test.go | 84 --------------------------------------------- 2 files changed, 76 insertions(+), 84 deletions(-) delete mode 100644 test/file_test.go diff --git a/provisioner_test.go b/provisioner_test.go index 6724887..dacca09 100644 --- a/provisioner_test.go +++ b/provisioner_test.go @@ -2,11 +2,15 @@ package astilectron import ( "context" + "fmt" "io/ioutil" + "log" "net/http/httptest" "os" "path/filepath" + "runtime" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -119,3 +123,75 @@ func TestNewDisembedderProvisioner(t *testing.T) { assert.NoError(t, err) testProvisionerSuccessful(t, *p, "linux", "amd64", DefaultVersionAstilectron, DefaultVersionElectron) } + +func TestZipShouldRemove(t *testing.T) { + l := log.New(log.Writer(), log.Prefix(), log.Flags()) + + appName := "Test" + dataDir, _ := filepath.Abs("./testdata/tmp") + vendorDir := filepath.Join(dataDir, "vendor") // https://github.com/CarsonSlovoka/go-astilectron/blob/e7796e5/paths.go#L56 + astilectronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("astilectron-v%s.zip", DefaultVersionAstilectron)) + astilectronDir := filepath.Join(vendorDir, fmt.Sprintf("astilectron")) + electronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s-v%s.zip", runtime.GOOS, runtime.GOARCH, DefaultVersionElectron)) + electronDir := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s", runtime.GOOS, runtime.GOARCH)) + + t.Log("Make sure the test directory doesn't exist.") + if err := os.RemoveAll(dataDir); err != nil && !os.IsNotExist(err) { + t.FailNow() + } + + t.Logf("Create a directory for test only:\n%s\n", dataDir) + if err := os.MkdirAll(dataDir, os.FileMode(0666)); err != nil { + t.Fatalf(err.Error()) + } + + defer func() { + t.Log("Remove the test directory after the test was done.") + if err := os.RemoveAll(dataDir); err != nil { + t.Fatalf(err.Error()) + } + }() + + { + t.Log("common process") + a, err := New(l, Options{ + AppName: appName, + DataDirectoryPath: dataDir, + }) + if err != nil { + t.Fatalf("main: creating astilectron failed: %s", err.Error()) + } + defer a.Close() + + a.HandleSignals() + + if err = a.Start(); err != nil { + t.Fatalf("main: starting astilectron failed: %s", err.Error()) + } + + // Stop the app immediately since we care about the file only to avoid access being denied. (delete) + a.Stop() + time.Sleep(10 * time.Second) + t.Log("astilectron Close") + } + + { + t.Log("Check UnZip successful") + if _, err := os.Stat(astilectronDir); os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + + if _, err := os.Stat(electronDir); os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + + t.Log("Check Zip doesn't exist") + if _, err := os.Stat(astilectronDownloadDst); !os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + + if _, err := os.Stat(electronDownloadDst); !os.IsNotExist(err) { + t.Fatalf(err.Error()) + } + } +} diff --git a/test/file_test.go b/test/file_test.go deleted file mode 100644 index 1f63e4f..0000000 --- a/test/file_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package main_test - -import ( - "fmt" - "github.com/asticode/go-astilectron" - "log" - "os" - "path/filepath" - "runtime" - "testing" - "time" -) - -func TestZipShouldRemove(t *testing.T) { - l := log.New(log.Writer(), log.Prefix(), log.Flags()) - - appName := "Test" - dataDir, _ := filepath.Abs("./temp/Test") - vendorDir := filepath.Join(dataDir, "vendor") // https://github.com/CarsonSlovoka/go-astilectron/blob/e7796e5/paths.go#L56 - astilectronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("astilectron-v%s.zip", astilectron.DefaultVersionAstilectron)) - astilectronDir := filepath.Join(vendorDir, fmt.Sprintf("astilectron")) - electronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s-v%s.zip", runtime.GOOS, runtime.GOARCH, astilectron.DefaultVersionElectron)) - electronDir := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s", runtime.GOOS, runtime.GOARCH)) - - t.Log("Make sure the test directory doesn't exist.") - if err := os.RemoveAll(dataDir); err != nil && !os.IsNotExist(err) { - t.FailNow() - } - - t.Logf("Create a directory for test only:\n%s\n", dataDir) - if err := os.MkdirAll(dataDir, os.FileMode(0666)); err != nil { - t.Fatalf(err.Error()) - } - - defer func() { - t.Log("Remove the test directory after the test was done.") - if err := os.RemoveAll(dataDir); err != nil { - t.Fatalf(err.Error()) - } - }() - - { - t.Log("common process") - a, err := astilectron.New(l, astilectron.Options{ - AppName: appName, - DataDirectoryPath: dataDir, - }) - if err != nil { - t.Fatalf("main: creating astilectron failed: %s", err.Error()) - } - defer a.Close() - - a.HandleSignals() - - if err = a.Start(); err != nil { - t.Fatalf("main: starting astilectron failed: %s", err.Error()) - } - - // Stop the app immediately since we care about the file only to avoid access being denied. (delete) - a.Stop() - time.Sleep(10 * time.Second) - t.Log("astilectron Close") - } - - { - t.Log("Check UnZip successful") - if _, err := os.Stat(astilectronDir); os.IsNotExist(err) { - t.Fatalf(err.Error()) - } - - if _, err := os.Stat(electronDir); os.IsNotExist(err) { - t.Fatalf(err.Error()) - } - - t.Log("Check Zip doesn't exist") - if _, err := os.Stat(astilectronDownloadDst); !os.IsNotExist(err) { - t.Fatalf(err.Error()) - } - - if _, err := os.Stat(electronDownloadDst); !os.IsNotExist(err) { - t.Fatalf(err.Error()) - } - } -} From 4bac7b9d74646f5eec8c4f79dee7a2754e9d241c Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 28 Mar 2022 09:50:28 +0800 Subject: [PATCH 04/10] check return value and closeFunc not nil --- provisioner.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/provisioner.go b/provisioner.go index 802d431..1c282ec 100644 --- a/provisioner.go +++ b/provisioner.go @@ -45,9 +45,9 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { } return func() (err error) { if err = os.Remove(p.AstilectronDownloadDst()); err != nil { - err = fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) + return fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) } - return + return nil }, err } dp.moverElectron = func(ctx context.Context, p Paths) (closeFunc func() error, err error) { @@ -56,9 +56,9 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { } return func() (err error) { if err = os.Remove(p.ElectronDownloadDst()); err != nil { - err = fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) + return fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) } - return + return nil }, err } return @@ -200,6 +200,9 @@ func (p *defaultProvisioner) provisionPackage(ctx context.Context, paths Paths, // Make sure to close defer func() { + if closeFunc == nil { + return + } if err := closeFunc(); err != nil { // Only log the error p.l.Error(fmt.Errorf("closing failed: %w", err)) @@ -356,9 +359,9 @@ func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron stri } return func() (err error) { if err = os.Remove(p.AstilectronDownloadDst()); err != nil { - err = fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) + return fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) } - return + return nil }, err } dp.moverElectron = func(ctx context.Context, p Paths) (closeFunc func() error, err error) { @@ -367,9 +370,9 @@ func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron stri } return func() (err error) { if err = os.Remove(p.ElectronDownloadDst()); err != nil { - err = fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) + return fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) } - return + return nil }, err } return dp From ae08b36c5a8f4b2b68add1fd5808ad5e8201c78a Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 28 Mar 2022 13:22:26 +0800 Subject: [PATCH 05/10] Use the methods already provided by go-astilectron to reduce duplication of code --- astilectron.go | 8 ++++ pkg/README.md | 1 + pkg/testing/log.go | 23 +++++++++++ provisioner_test.go | 98 ++++++++++++++++----------------------------- 4 files changed, 66 insertions(+), 64 deletions(-) create mode 100644 pkg/README.md create mode 100644 pkg/testing/log.go diff --git a/astilectron.go b/astilectron.go index bbfb68f..0f64b18 100644 --- a/astilectron.go +++ b/astilectron.go @@ -347,6 +347,14 @@ func (a *Astilectron) watchCmd(cmd *exec.Cmd) { }) } +func (a *Astilectron) GetVersionAstilectron() string { + return a.options.VersionAstilectron +} + +func (a *Astilectron) GetVersionElectron() string { + return a.options.VersionElectron +} + // Close closes Astilectron properly func (a *Astilectron) Close() { a.l.Debug("Closing...") diff --git a/pkg/README.md b/pkg/README.md new file mode 100644 index 0000000..4fc6b53 --- /dev/null +++ b/pkg/README.md @@ -0,0 +1 @@ +The architecture of the directory same as the [go.src](https://github.com/golang/go/tree/ad646b3/src) \ No newline at end of file diff --git a/pkg/testing/log.go b/pkg/testing/log.go new file mode 100644 index 0000000..d66d5ed --- /dev/null +++ b/pkg/testing/log.go @@ -0,0 +1,23 @@ +package testing + +import ( + "fmt" + "testing" +) + +type StdLogger struct { + *testing.T +} + +func (t StdLogger) Print(v ...interface{}) { + fmt.Print(v) +} + +func (t StdLogger) Printf(format string, v ...interface{}) { + fmt.Printf(format, v) +} + +// AdaptTestLogger transforms an testing.T into a StdLogger if needed +func AdaptTestLogger(t *testing.T) StdLogger { + return StdLogger{t} +} diff --git a/provisioner_test.go b/provisioner_test.go index dacca09..2aae910 100644 --- a/provisioner_test.go +++ b/provisioner_test.go @@ -2,17 +2,13 @@ package astilectron import ( "context" - "fmt" + testingHelper "github.com/asticode/go-astilectron/pkg/testing" + "github.com/stretchr/testify/assert" "io/ioutil" - "log" "net/http/httptest" "os" "path/filepath" - "runtime" "testing" - "time" - - "github.com/stretchr/testify/assert" ) func testProvisionerSuccessful(t *testing.T, p Paths, osName, arch, versionAstilectron, versionElectron string) { @@ -125,73 +121,47 @@ func TestNewDisembedderProvisioner(t *testing.T) { } func TestZipShouldRemove(t *testing.T) { - l := log.New(log.Writer(), log.Prefix(), log.Flags()) - - appName := "Test" - dataDir, _ := filepath.Abs("./testdata/tmp") - vendorDir := filepath.Join(dataDir, "vendor") // https://github.com/CarsonSlovoka/go-astilectron/blob/e7796e5/paths.go#L56 - astilectronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("astilectron-v%s.zip", DefaultVersionAstilectron)) - astilectronDir := filepath.Join(vendorDir, fmt.Sprintf("astilectron")) - electronDownloadDst := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s-v%s.zip", runtime.GOOS, runtime.GOARCH, DefaultVersionElectron)) - electronDir := filepath.Join(vendorDir, fmt.Sprintf("electron-%s-%s", runtime.GOOS, runtime.GOARCH)) - - t.Log("Make sure the test directory doesn't exist.") - if err := os.RemoveAll(dataDir); err != nil && !os.IsNotExist(err) { + var o = Options{ + DataDirectoryPath: mockedTempPath(), + } + + // Make sure the test directory doesn't exist. + if err := os.RemoveAll(o.DataDirectoryPath); err != nil && !os.IsNotExist(err) { t.FailNow() } + defer os.RemoveAll(o.DataDirectoryPath) + + a, err := New(&testingHelper.StdLogger{T: t}, o) + if err != nil { + t.Fatalf("main: creating astilectron failed: %s", err.Error()) + } - t.Logf("Create a directory for test only:\n%s\n", dataDir) - if err := os.MkdirAll(dataDir, os.FileMode(0666)); err != nil { - t.Fatalf(err.Error()) + p := a.Paths() + astilectronDir := p.astilectronDownloadDst[0 : len(p.astilectronDownloadDst)-len(a.GetVersionAstilectron())-6] // 6: -v.zip // astilectron-v%s.zip => astilectron + electronDir := p.electronDownloadDst[0 : len(p.electronDownloadDst)-len(a.GetVersionElectron())-6] // electron-%s-%s-v%s.zip => electron-%s-%s + if err != nil { + t.FailNow() } - defer func() { - t.Log("Remove the test directory after the test was done.") - if err := os.RemoveAll(dataDir); err != nil { - t.Fatalf(err.Error()) - } - }() - - { - t.Log("common process") - a, err := New(l, Options{ - AppName: appName, - DataDirectoryPath: dataDir, - }) - if err != nil { - t.Fatalf("main: creating astilectron failed: %s", err.Error()) - } - defer a.Close() - - a.HandleSignals() - - if err = a.Start(); err != nil { - t.Fatalf("main: starting astilectron failed: %s", err.Error()) - } - - // Stop the app immediately since we care about the file only to avoid access being denied. (delete) - a.Stop() - time.Sleep(10 * time.Second) - t.Log("astilectron Close") + if err = a.provision(); err != nil { + t.FailNow() } - { - t.Log("Check UnZip successful") - if _, err := os.Stat(astilectronDir); os.IsNotExist(err) { - t.Fatalf(err.Error()) - } + // Check UnZip successful + if _, err := os.Stat(astilectronDir); os.IsNotExist(err) { + t.Fatalf("%v", err) + } - if _, err := os.Stat(electronDir); os.IsNotExist(err) { - t.Fatalf(err.Error()) - } + if _, err := os.Stat(electronDir); os.IsNotExist(err) { + t.Fatalf("%v", err) + } - t.Log("Check Zip doesn't exist") - if _, err := os.Stat(astilectronDownloadDst); !os.IsNotExist(err) { - t.Fatalf(err.Error()) - } + // Check Zip doesn't exist + if _, err := os.Stat(p.AstilectronDownloadDst()); !os.IsNotExist(err) { + t.Fatalf("%v", err) + } - if _, err := os.Stat(electronDownloadDst); !os.IsNotExist(err) { - t.Fatalf(err.Error()) - } + if _, err := os.Stat(p.ElectronDownloadDst()); !os.IsNotExist(err) { + t.Fatalf("%v", err) } } From 3dd1a9668b01a4e488bb8de10c75fe43d019a883 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 28 Mar 2022 14:16:21 +0800 Subject: [PATCH 06/10] use AstilectronDirectory() and ElectronDirectory() --- provisioner_test.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/provisioner_test.go b/provisioner_test.go index 2aae910..c94561e 100644 --- a/provisioner_test.go +++ b/provisioner_test.go @@ -137,22 +137,17 @@ func TestZipShouldRemove(t *testing.T) { } p := a.Paths() - astilectronDir := p.astilectronDownloadDst[0 : len(p.astilectronDownloadDst)-len(a.GetVersionAstilectron())-6] // 6: -v.zip // astilectron-v%s.zip => astilectron - electronDir := p.electronDownloadDst[0 : len(p.electronDownloadDst)-len(a.GetVersionElectron())-6] // electron-%s-%s-v%s.zip => electron-%s-%s - if err != nil { - t.FailNow() - } if err = a.provision(); err != nil { t.FailNow() } // Check UnZip successful - if _, err := os.Stat(astilectronDir); os.IsNotExist(err) { + if _, err := os.Stat(p.AstilectronDirectory()); os.IsNotExist(err) { t.Fatalf("%v", err) } - if _, err := os.Stat(electronDir); os.IsNotExist(err) { + if _, err := os.Stat(p.ElectronDirectory()); os.IsNotExist(err) { t.Fatalf("%v", err) } From 79222481c3e714efa25ddb81c4325948f97708b5 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 28 Mar 2022 18:19:09 +0800 Subject: [PATCH 07/10] Bumped astikit --- astilectron.go | 8 -------- go.mod | 2 +- go.sum | 4 ++-- pkg/README.md | 1 - pkg/testing/log.go | 23 ----------------------- provisioner_test.go | 4 ++-- 6 files changed, 5 insertions(+), 37 deletions(-) delete mode 100644 pkg/README.md delete mode 100644 pkg/testing/log.go diff --git a/astilectron.go b/astilectron.go index 0f64b18..bbfb68f 100644 --- a/astilectron.go +++ b/astilectron.go @@ -347,14 +347,6 @@ func (a *Astilectron) watchCmd(cmd *exec.Cmd) { }) } -func (a *Astilectron) GetVersionAstilectron() string { - return a.options.VersionAstilectron -} - -func (a *Astilectron) GetVersionElectron() string { - return a.options.VersionElectron -} - // Close closes Astilectron properly func (a *Astilectron) Close() { a.l.Debug("Closing...") diff --git a/go.mod b/go.mod index e611841..f19b230 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,6 @@ module github.com/asticode/go-astilectron go 1.13 require ( - github.com/asticode/go-astikit v0.15.0 + github.com/asticode/go-astikit v0.29.1 github.com/stretchr/testify v1.4.0 ) diff --git a/go.sum b/go.sum index 8bdd880..21a6bcf 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/asticode/go-astikit v0.15.0 h1:mpy55njSQ9SS9gP9vmPAfo08YpRt+4pr5/1emK6DFF0= -github.com/asticode/go-astikit v0.15.0/go.mod h1:h4ly7idim1tNhaVkdVBeXQZEE3L0xblP7fCWbgwipF0= +github.com/asticode/go-astikit v0.29.1 h1:w27sLYXK84mDwArf/Vw1BiD5dfD5PBDB+iHoIcpYq0w= +github.com/asticode/go-astikit v0.29.1/go.mod h1:h4ly7idim1tNhaVkdVBeXQZEE3L0xblP7fCWbgwipF0= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/pkg/README.md b/pkg/README.md deleted file mode 100644 index 4fc6b53..0000000 --- a/pkg/README.md +++ /dev/null @@ -1 +0,0 @@ -The architecture of the directory same as the [go.src](https://github.com/golang/go/tree/ad646b3/src) \ No newline at end of file diff --git a/pkg/testing/log.go b/pkg/testing/log.go deleted file mode 100644 index d66d5ed..0000000 --- a/pkg/testing/log.go +++ /dev/null @@ -1,23 +0,0 @@ -package testing - -import ( - "fmt" - "testing" -) - -type StdLogger struct { - *testing.T -} - -func (t StdLogger) Print(v ...interface{}) { - fmt.Print(v) -} - -func (t StdLogger) Printf(format string, v ...interface{}) { - fmt.Printf(format, v) -} - -// AdaptTestLogger transforms an testing.T into a StdLogger if needed -func AdaptTestLogger(t *testing.T) StdLogger { - return StdLogger{t} -} diff --git a/provisioner_test.go b/provisioner_test.go index c94561e..a88d57f 100644 --- a/provisioner_test.go +++ b/provisioner_test.go @@ -2,7 +2,7 @@ package astilectron import ( "context" - testingHelper "github.com/asticode/go-astilectron/pkg/testing" + "github.com/asticode/go-astikit" "github.com/stretchr/testify/assert" "io/ioutil" "net/http/httptest" @@ -131,7 +131,7 @@ func TestZipShouldRemove(t *testing.T) { } defer os.RemoveAll(o.DataDirectoryPath) - a, err := New(&testingHelper.StdLogger{T: t}, o) + a, err := New(astikit.AdaptTestLogger(t), o) if err != nil { t.Fatalf("main: creating astilectron failed: %s", err.Error()) } From 9a97a01ba9c9f077f384f94b2ef9943e250d2ab8 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 28 Mar 2022 18:27:17 +0800 Subject: [PATCH 08/10] Add Debugf for delete the zip of {ElectronDownloadDst, AstilectronDownloadDst} --- provisioner.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/provisioner.go b/provisioner.go index 1c282ec..ab28ee6 100644 --- a/provisioner.go +++ b/provisioner.go @@ -47,6 +47,7 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { if err = os.Remove(p.AstilectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) } + dp.l.Debugf("Remove download file: %s", p.AstilectronDownloadDst()) return nil }, err } @@ -58,6 +59,7 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { if err = os.Remove(p.ElectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) } + dp.l.Debugf("Remove download file: %s", p.ElectronDownloadDst()) return nil }, err } @@ -361,6 +363,7 @@ func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron stri if err = os.Remove(p.AstilectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) } + dp.l.Debugf("Remove download file: %s", p.AstilectronDownloadDst()) return nil }, err } @@ -372,6 +375,7 @@ func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron stri if err = os.Remove(p.ElectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) } + dp.l.Debugf("Remove download file: %s", p.ElectronDownloadDst()) return nil }, err } From 3481a4e396ea2fa4cef5282d957edcc4dd50505b Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 29 Mar 2022 09:45:07 +0800 Subject: [PATCH 09/10] Display the debug log before removing. --- provisioner.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/provisioner.go b/provisioner.go index ab28ee6..2a2dcb0 100644 --- a/provisioner.go +++ b/provisioner.go @@ -44,10 +44,10 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { return nil, fmt.Errorf("downloading %s into %s failed: %w", p.AstilectronDownloadSrc(), p.AstilectronDownloadDst(), err) } return func() (err error) { + dp.l.Debugf("removing %s", p.AstilectronDownloadDst()) if err = os.Remove(p.AstilectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) } - dp.l.Debugf("Remove download file: %s", p.AstilectronDownloadDst()) return nil }, err } @@ -56,10 +56,10 @@ func newDefaultProvisioner(l astikit.StdLogger) (dp *defaultProvisioner) { return nil, fmt.Errorf("downloading %s into %s failed: %w", p.ElectronDownloadSrc(), p.ElectronDownloadDst(), err) } return func() (err error) { + dp.l.Debugf("removing %s", p.ElectronDownloadDst()) if err = os.Remove(p.ElectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) } - dp.l.Debugf("Remove download file: %s", p.ElectronDownloadDst()) return nil }, err } @@ -360,10 +360,10 @@ func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron stri return nil, fmt.Errorf("disembedding %s into %s failed: %w", pathAstilectron, p.AstilectronDownloadDst(), err) } return func() (err error) { + dp.l.Debugf("removing %s", p.AstilectronDownloadDst()) if err = os.Remove(p.AstilectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.AstilectronDownloadDst(), err) } - dp.l.Debugf("Remove download file: %s", p.AstilectronDownloadDst()) return nil }, err } @@ -372,10 +372,10 @@ func NewDisembedderProvisioner(d Disembedder, pathAstilectron, pathElectron stri return nil, fmt.Errorf("disembedding %s into %s failed: %w", pathElectron, p.ElectronDownloadDst(), err) } return func() (err error) { + dp.l.Debugf("removing %s", p.ElectronDownloadDst()) if err = os.Remove(p.ElectronDownloadDst()); err != nil { return fmt.Errorf("removing %s failed: %w", p.ElectronDownloadDst(), err) } - dp.l.Debugf("Remove download file: %s", p.ElectronDownloadDst()) return nil }, err } From 6daec70338071bbd144191b10d6b33d200e1a142 Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 29 Mar 2022 10:25:39 +0800 Subject: [PATCH 10/10] Word optimization --- provisioner_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/provisioner_test.go b/provisioner_test.go index a88d57f..c62e69e 100644 --- a/provisioner_test.go +++ b/provisioner_test.go @@ -120,26 +120,26 @@ func TestNewDisembedderProvisioner(t *testing.T) { testProvisionerSuccessful(t, *p, "linux", "amd64", DefaultVersionAstilectron, DefaultVersionElectron) } -func TestZipShouldRemove(t *testing.T) { +func TestRemoveDownloadDst(t *testing.T) { var o = Options{ DataDirectoryPath: mockedTempPath(), } // Make sure the test directory doesn't exist. if err := os.RemoveAll(o.DataDirectoryPath); err != nil && !os.IsNotExist(err) { - t.FailNow() + t.Fatalf("main: removing %s failed: %s", o.DataDirectoryPath, err) } defer os.RemoveAll(o.DataDirectoryPath) a, err := New(astikit.AdaptTestLogger(t), o) if err != nil { - t.Fatalf("main: creating astilectron failed: %s", err.Error()) + t.Fatalf("main: creating astilectron failed: %s", err) } p := a.Paths() if err = a.provision(); err != nil { - t.FailNow() + t.Fatalf("main: provisionning failed: %s", err) } // Check UnZip successful