From d61ba18f8a4a1aa98697da0e50730296870e2a53 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 6 Jun 2024 11:55:22 +0300 Subject: [PATCH] Fix require warning with tests from stdin --- js/bundle_test.go | 28 ++++++++++++++++++++++++++++ js/modules/require_impl.go | 32 ++++++++++++++++++++------------ js/path_resolution_test.go | 34 ++++++++++++++++++++++++++++++++++ js/tc39/tc39_test.go | 1 + 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 6bdc191443af..c077e23fca8e 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -73,6 +73,34 @@ func getSimpleBundle(tb testing.TB, filename, data string, opts ...interface{}) ) } +func getSimpleBundleStdin(tb testing.TB, pwd *url.URL, data string, opts ...interface{}) (*Bundle, error) { + fs := fsext.NewMemMapFs() + var rtOpts *lib.RuntimeOptions + var logger logrus.FieldLogger + for _, o := range opts { + switch opt := o.(type) { + case fsext.Fs: + fs = opt + case lib.RuntimeOptions: + rtOpts = &opt + case logrus.FieldLogger: + logger = opt + default: + tb.Fatalf("unknown test option %q", opt) + } + } + + return NewBundle( + getTestPreInitState(tb, logger, rtOpts), + &loader.SourceData{ + URL: &url.URL{Path: "/-", Scheme: "file"}, + Data: []byte(data), + PWD: pwd, + }, + map[string]fsext.Fs{"file": fs, "https": fsext.NewMemMapFs()}, + ) +} + func TestNewBundle(t *testing.T) { t.Parallel() t.Run("Blank", func(t *testing.T) { diff --git a/js/modules/require_impl.go b/js/modules/require_impl.go index 0f0833eaaac2..4b091a0447aa 100644 --- a/js/modules/require_impl.go +++ b/js/modules/require_impl.go @@ -49,16 +49,16 @@ func (r *LegacyRequireImpl) Require(specifier string) (*goja.Object, error) { defer func() { r.currentlyRequiredModule = currentPWD }() + fileURL, err := loader.Resolve(r.currentlyRequiredModule, specifier) + if err != nil { + return nil, err + } // In theory we can give that downwards, but this makes the code more tightly coupled // plus as explained above this will be removed in the future so the code reflects more // closely what will be needed then - if !r.modules.resolver.locked { + if fileURL.Scheme == "file" && !r.modules.resolver.locked { r.warnUserOnPathResolutionDifferences(specifier) } - fileURL, err := loader.Resolve(r.currentlyRequiredModule, specifier) - if err != nil { - return nil, err - } r.currentlyRequiredModule = loader.Dir(fileURL) } @@ -84,9 +84,8 @@ func (r *LegacyRequireImpl) warnUserOnPathResolutionDifferences(specifier string return loader.Dir(u), nil } // Warn users on their require depending on the none standard k6 behaviour. - rt := r.vu.Runtime() logger := r.vu.InitEnv().Logger - correct, err := normalizePathToURL(getCurrentModuleScript(rt)) + correct, err := normalizePathToURL(getCurrentModuleScript(r.vu)) if err != nil { logger.Warningf("Couldn't get the \"correct\" path to resolve specifier %q against: %q"+ "Please report to issue %s. "+ @@ -99,7 +98,7 @@ func (r *LegacyRequireImpl) warnUserOnPathResolutionDifferences(specifier string correct, r.currentlyRequiredModule, specifier, issueLink) } - k6behaviourString, err := getPreviousRequiringFile(rt) + k6behaviourString, err := getPreviousRequiringFile(r.vu) if err != nil { logger.Warningf("Couldn't get the \"wrong\" path to resolve specifier %q against: %q"+ "Please report to issue %s. "+ @@ -124,16 +123,21 @@ func (r *LegacyRequireImpl) warnUserOnPathResolutionDifferences(specifier string } } -func getCurrentModuleScript(rt *goja.Runtime) string { +func getCurrentModuleScript(vu VU) string { + rt := vu.Runtime() var parent string var buf [2]goja.StackFrame frames := rt.CaptureCallStack(2, buf[:0]) + if len(frames) == 0 || frames[1].SrcName() == "file:///-" { + return vu.InitEnv().CWD.JoinPath("./-").String() + } parent = frames[1].SrcName() return parent } -func getPreviousRequiringFile(rt *goja.Runtime) (string, error) { +func getPreviousRequiringFile(vu VU) (string, error) { + rt := vu.Runtime() var buf [1000]goja.StackFrame frames := rt.CaptureCallStack(1000, buf[:0]) @@ -141,7 +145,11 @@ func getPreviousRequiringFile(rt *goja.Runtime) (string, error) { // TODO have this precalculated automatically if frame.FuncName() == "go.k6.io/k6/js.(*requireImpl).require-fm" { // we need to get the one *before* but as we skip the first one the index matches ;) - return frames[i].SrcName(), nil + result := frames[i].SrcName() + if result == "file:///-" { + return vu.InitEnv().CWD.JoinPath("./-").String(), nil + } + return result, nil } } // hopefully nobody is calling `require` with 1000 big stack :crossedfingers: @@ -150,5 +158,5 @@ func getPreviousRequiringFile(rt *goja.Runtime) (string, error) { } // fallback - return frames[len(frames)-1].SrcName(), nil + return vu.InitEnv().CWD.JoinPath("./-").String(), nil } diff --git a/js/path_resolution_test.go b/js/path_resolution_test.go index 6b6f01a89d23..2ea156106e34 100644 --- a/js/path_resolution_test.go +++ b/js/path_resolution_test.go @@ -2,6 +2,7 @@ package js import ( "context" + "net/url" "testing" "github.com/sirupsen/logrus" @@ -218,6 +219,39 @@ func TestRequirePathResolution(t *testing.T) { } }) } + + pwd, err := url.Parse("file:///A/A/A/A/") + require.NoError(t, err) + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run("STDIN-"+name, func(t *testing.T) { + t.Parallel() + fs := fsext.NewMemMapFs() + err := writeToFs(fs, testCase.fsMap) + fs = fsext.NewCacheOnReadFs(fs, fsext.NewMemMapFs(), 0) + require.NoError(t, err) + logger, hook := testutils.NewLoggerWithHook(t, logrus.WarnLevel) + + b, err := getSimpleBundleStdin(t, pwd, testCase.fsMap["/A/A/A/A/script.js"].(string), fs, logger) + require.NoError(t, err) + + _, err = b.Instantiate(context.Background(), 0) + require.NoError(t, err) + logs := hook.Drain() + + if len(testCase.expectedLogs) == 0 { + require.Empty(t, logs) + return + } + require.Equal(t, len(logs), len(testCase.expectedLogs)) + + for i, log := range logs { + require.Contains(t, log.Message, testCase.expectedLogs[i], "log line %d", i) + } + }) + } } // writeToFs is a small helper to write a map of paths to contents to the filesystem provided. diff --git a/js/tc39/tc39_test.go b/js/tc39/tc39_test.go index 56ab0419886f..be22038f9817 100644 --- a/js/tc39/tc39_test.go +++ b/js/tc39/tc39_test.go @@ -710,6 +710,7 @@ func (ctx *tc39TestCtx) runTC39Module(name, src string, includes []string, vm *g ms := modules.NewModuleSystem(mr, moduleRuntime.VU) impl := modules.NewLegacyRequireImpl(moduleRuntime.VU, ms, *base) require.NoError(ctx.t, vm.Set("require", impl.Require)) + moduleRuntime.VU.InitEnvField.CWD = base early = false _, err = ms.RunSourceData(&loader.SourceData{