From fa8dfe1ff9bb5ede54215a8da1a548de2a234d6f Mon Sep 17 00:00:00 2001 From: oleiade Date: Tue, 7 Nov 2023 17:12:24 +0100 Subject: [PATCH] Apply suggestions from code review --- js/modules/k6/experimental/fs/cache.go | 12 ++++++------ js/modules/k6/experimental/fs/module.go | 11 ----------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/js/modules/k6/experimental/fs/cache.go b/js/modules/k6/experimental/fs/cache.go index 2602d2821d2d..c7b9f41fc769 100644 --- a/js/modules/k6/experimental/fs/cache.go +++ b/js/modules/k6/experimental/fs/cache.go @@ -6,7 +6,7 @@ import ( "path/filepath" "sync" - "github.com/spf13/afero" + "go.k6.io/k6/lib/fsext" ) // cache is a cache of opened files. @@ -40,14 +40,14 @@ type cache struct { // // Parameters: // - filename: The name of the file to be retrieved. This should be a relative or absolute path. -// - fromFs: The filesystem (from the afero package) from which the file should be read if not already cached. +// - fromFs: The filesystem (from the fsext package) from which the file should be read if not already cached. // // Returns: // - A byte slice containing the content of the specified file. // - An error if there's any issue opening or reading the file. If the file content is // successfully cached and returned once, subsequent calls will not produce // file-related errors for the same file, as the cached value will be used. -func (fr *cache) open(filename string, fromFs afero.Fs) (data []byte, err error) { +func (fr *cache) open(filename string, fromFs fsext.Fs) (data []byte, err error) { filename = filepath.Clean(filename) if f, ok := fr.openedFiles.Load(filename); ok { @@ -59,9 +59,9 @@ func (fr *cache) open(filename string, fromFs afero.Fs) (data []byte, err error) return data, nil } - // The underlying afero.Fs.Open method will cache the file content during this + // The underlying fsext.Fs.Open method will cache the file content during this // operation. Which will lead to effectively holding the content of the file in memory twice. - // However, as per #1079, we plan to eventually reduce our dependency on afero, and + // However, as per #1079, we plan to eventually reduce our dependency on fsext, and // expect this issue to be resolved at that point. // TODO: re-evaluate opening from the FS this once #1079 is resolved. f, err := fromFs.Open(filename) @@ -77,7 +77,7 @@ func (fr *cache) open(filename string, fromFs afero.Fs) (data []byte, err error) data, err = io.ReadAll(f) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read the content of file %s: %w", filename, err) } fr.openedFiles.Store(filename, data) diff --git a/js/modules/k6/experimental/fs/module.go b/js/modules/k6/experimental/fs/module.go index faf1b87105f2..cf5964adc872 100644 --- a/js/modules/k6/experimental/fs/module.go +++ b/js/modules/k6/experimental/fs/module.go @@ -193,11 +193,6 @@ func (f *File) Read(into goja.Value) *goja.Promise { return promise } - // We expect the into argument to be a `Uint8Array` instance - - // intoObj := into.ToObject(f.vu.Runtime()) - // uint8ArrayConstructor := f.vu.Runtime().Get("Uint8Array") - // if isUint8Array := intoObj.Get("constructor").SameAs(uint8ArrayConstructor); !isUint8Array { intoObj := into.ToObject(f.vu.Runtime()) if !isUint8Array(f.vu.Runtime(), intoObj) { reject(newFsError(TypeError, "read() failed; reason: into argument must be a Uint8Array")) @@ -233,12 +228,6 @@ func (f *File) Read(into goja.Value) *goja.Promise { return nil } - // The [file.Read] method will return an EOFError as soon as it reached - // the end of the file. - // - // However, following deno's behavior, we express - // EOF to users by returning null, when and only when there aren't any - // more bytes to read. var fsErr *fsError isFSErr := errors.As(readErr, &fsErr)