Skip to content

Commit

Permalink
Fix require warning with tests from stdin
Browse files Browse the repository at this point in the history
  • Loading branch information
mstoykov committed Jun 7, 2024
1 parent 75615bc commit fe7fd71
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 12 deletions.
28 changes: 28 additions & 0 deletions js/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
32 changes: 20 additions & 12 deletions js/modules/require_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ func (r *LegacyRequireImpl) Require(specifier string) (*sobek.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)
}

Expand All @@ -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. "+
Expand All @@ -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. "+
Expand All @@ -124,24 +123,33 @@ func (r *LegacyRequireImpl) warnUserOnPathResolutionDifferences(specifier string
}
}

func getCurrentModuleScript(rt *sobek.Runtime) string {
func getCurrentModuleScript(vu VU) string {
rt := vu.Runtime()
var parent string
var buf [2]sobek.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 *sobek.Runtime) (string, error) {
func getPreviousRequiringFile(vu VU) (string, error) {
rt := vu.Runtime()
var buf [1000]sobek.StackFrame
frames := rt.CaptureCallStack(1000, buf[:0])

for i, frame := range frames[1:] { // first one should be the current require
// 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:
Expand All @@ -150,5 +158,5 @@ func getPreviousRequiringFile(rt *sobek.Runtime) (string, error) {
}

// fallback
return frames[len(frames)-1].SrcName(), nil
return vu.InitEnv().CWD.JoinPath("./-").String(), nil
}
34 changes: 34 additions & 0 deletions js/path_resolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package js

import (
"context"
"net/url"
"testing"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions js/tc39/tc39_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ func (ctx *tc39TestCtx) runTC39Module(name, src string, includes []string, vm *s
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{
Expand Down

0 comments on commit fe7fd71

Please sign in to comment.