Skip to content

remove the options package, simplify constructors #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
robbyt merged 4 commits into from
Apr 11, 2025
Merged

remove the options package, simplify constructors #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
63fb0d4
add more coverage
robbyt Apr 11, 2025
3036a86
lots of cleanup after removal of the options package
robbyt Apr 11, 2025
54bca15
adjust polyscript.go docs
robbyt Apr 11, 2025
2e0f30e
add tests for the machine's top-level new.go files
robbyt Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 31 additions & 49 deletions engine/benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ import (
"testing"

"github.com/robbyt/go-polyscript"
"github.com/robbyt/go-polyscript/engine/options"
"github.com/robbyt/go-polyscript/execution/constants"
"github.com/robbyt/go-polyscript/execution/data"
risorCompiler "github.com/robbyt/go-polyscript/machines/risor/compiler"
starlarkCompiler "github.com/robbyt/go-polyscript/machines/starlark/compiler"
)

// quietHandler is a slog.Handler that discards all logs
Expand Down Expand Up @@ -63,15 +59,12 @@ func BenchmarkEvaluationPatterns(b *testing.B) {
inputData := map[string]any{
"name": "World",
}
dataProvider := data.NewStaticProvider(inputData)

// Create and evaluate in each iteration (simulating one-time use)
evaluator, err := polyscript.FromRisorString(
evaluator, err := polyscript.FromRisorStringWithData(
scriptContent,
options.WithDefaults(),
options.WithDataProvider(dataProvider),
options.WithLogHandler(quietHandler),
risorCompiler.WithGlobals([]string{constants.Ctx}),
inputData,
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create evaluator: %v", err)
Expand All @@ -87,15 +80,13 @@ func BenchmarkEvaluationPatterns(b *testing.B) {

b.Run("CompileOnceRunMany", func(b *testing.B) {
// Create evaluator once, outside the loop
dataProvider := data.NewStaticProvider(map[string]any{
inputData := map[string]any{
"name": "World",
})
evaluator, err := polyscript.FromRisorString(
}
evaluator, err := polyscript.FromRisorStringWithData(
scriptContent,
options.WithDefaults(),
options.WithDataProvider(dataProvider),
options.WithLogHandler(quietHandler),
risorCompiler.WithGlobals([]string{constants.Ctx}),
inputData,
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create evaluator: %v", err)
Expand Down Expand Up @@ -133,13 +124,11 @@ func BenchmarkDataProviders(b *testing.B) {
}

b.Run("StaticProvider", func(b *testing.B) {
dataProvider := data.NewStaticProvider(inputData)
evaluator, err := polyscript.FromRisorString(
// Using the *WithData version of the function which sets up a StaticProvider
evaluator, err := polyscript.FromRisorStringWithData(
scriptContent,
options.WithDefaults(),
options.WithDataProvider(dataProvider),
options.WithLogHandler(quietHandler),
risorCompiler.WithGlobals([]string{constants.Ctx}),
inputData,
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create evaluator: %v", err)
Expand All @@ -155,13 +144,10 @@ func BenchmarkDataProviders(b *testing.B) {
})

b.Run("ContextProvider", func(b *testing.B) {
dataProvider := data.NewContextProvider(constants.EvalData)
// Using the standard version which uses a ContextProvider
evaluator, err := polyscript.FromRisorString(
scriptContent,
options.WithDefaults(),
options.WithDataProvider(dataProvider),
options.WithLogHandler(quietHandler),
risorCompiler.WithGlobals([]string{constants.Ctx}),
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create evaluator: %v", err)
Expand All @@ -179,22 +165,23 @@ func BenchmarkDataProviders(b *testing.B) {
})

b.Run("CompositeProvider", func(b *testing.B) {
staticProvider := data.NewStaticProvider(map[string]any{"defaultKey": "value"})
contextProvider := data.NewContextProvider(constants.EvalData)
compositeProvider := data.NewCompositeProvider(contextProvider, staticProvider)

evaluator, err := polyscript.FromRisorString(
// For CompositeProvider use case, we can prepare the context separately
staticData := map[string]any{"defaultKey": "value"}
evaluator, err := polyscript.FromRisorStringWithData(
scriptContent,
options.WithDefaults(),
options.WithDataProvider(compositeProvider),
options.WithLogHandler(quietHandler),
risorCompiler.WithGlobals([]string{constants.Ctx}),
staticData, // Static part
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create evaluator: %v", err)
}

ctx := context.WithValue(context.Background(), constants.EvalData, inputData)
ctx := context.Background()
// Use PrepareContext to add the dynamic part
ctx, err = evaluator.PrepareContext(ctx, inputData)
if err != nil {
b.Fatalf("Failed to prepare context: %v", err)
}

b.ResetTimer()
for i := 0; i < b.N; i++ {
Expand All @@ -215,7 +202,6 @@ func BenchmarkVMComparison(b *testing.B) {
inputData := map[string]any{
"name": "World",
}
staticProvider := data.NewStaticProvider(inputData)

// Risor script
risorScript := `
Expand All @@ -240,12 +226,10 @@ message = "Hello, " + name + "!"
// which is more complex to set up in this benchmark template

b.Run("RisorVM", func(b *testing.B) {
evaluator, err := polyscript.FromRisorString(
evaluator, err := polyscript.FromRisorStringWithData(
risorScript,
options.WithDefaults(),
options.WithDataProvider(staticProvider),
options.WithLogHandler(quietHandler),
risorCompiler.WithGlobals([]string{constants.Ctx}),
inputData,
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create Risor evaluator: %v", err)
Expand All @@ -261,12 +245,10 @@ message = "Hello, " + name + "!"
})

b.Run("StarlarkVM", func(b *testing.B) {
evaluator, err := polyscript.FromStarlarkString(
evaluator, err := polyscript.FromStarlarkStringWithData(
starlarkScript,
options.WithDefaults(),
options.WithDataProvider(staticProvider),
options.WithLogHandler(quietHandler),
starlarkCompiler.WithGlobals([]string{constants.Ctx}),
inputData,
quietHandler,
)
if err != nil {
b.Fatalf("Failed to create Starlark evaluator: %v", err)
Expand Down
83 changes: 16 additions & 67 deletions engine/evaluator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,8 @@ import (

"github.com/robbyt/go-polyscript"
"github.com/robbyt/go-polyscript/engine"
"github.com/robbyt/go-polyscript/engine/options"
"github.com/robbyt/go-polyscript/execution/constants"
"github.com/robbyt/go-polyscript/execution/data"
"github.com/robbyt/go-polyscript/machines/mocks"
risorCompiler "github.com/robbyt/go-polyscript/machines/risor/compiler"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -114,18 +111,16 @@ func TestEvalDataPreparerInterface(t *testing.T) {
// Create a logger for testing
handler := slog.NewTextHandler(os.Stdout, nil)

// Create a ContextProvider for this test
provider := data.NewContextProvider(constants.EvalData)

// Create an evaluator with PrepareContext capability
evaluator, err := polyscript.FromRisorString(`
// The key name may be different in the new implementation
scriptData := map[string]any{"greeting": "Hello, World!"}
evaluator, err := polyscript.FromRisorStringWithData(`
method := ctx["request"]["Method"]
greeting := ctx["input_data"]["greeting"]
greeting := ctx["greeting"] // With new implementation, keys are at top level
method + " " + greeting
`,
options.WithLogHandler(handler),
options.WithDataProvider(provider),
risorCompiler.WithGlobals([]string{constants.Ctx}),
scriptData,
handler,
)
require.NoError(t, err)
require.NotNil(t, evaluator)
Expand All @@ -134,28 +129,12 @@ method + " " + greeting
ctx := context.Background()
req, err := http.NewRequest("GET", "http://localhost/test", nil)
require.NoError(t, err)
scriptData := map[string]any{"greeting": "Hello, World!"}

// Use PrepareContext to enrich the context
enrichedCtx, err := evaluator.PrepareContext(ctx, req, scriptData)
enrichedCtx, err := evaluator.PrepareContext(ctx, req)
require.NoError(t, err)
require.NotNil(t, enrichedCtx)

// Verify data was stored in context
storedData, ok := enrichedCtx.Value(constants.EvalData).(map[string]any)
require.True(t, ok, "Data should be stored in context")
require.NotNil(t, storedData, "Stored data should not be nil")

// Verify request data
requestData, ok := storedData[constants.Request].(map[string]any)
require.True(t, ok, "Request data should be available")
assert.Equal(t, "GET", requestData["Method"], "Request method should be stored")

// Verify script data
scriptDataStored, ok := storedData[constants.InputData].(map[string]any)
require.True(t, ok, "input_data should be available")
assert.Equal(t, "Hello, World!", scriptDataStored["greeting"], "Greeting should be stored")

// Test evaluation with the enriched context
result, err := evaluator.Eval(enrichedCtx)
require.NoError(t, err)
Expand Down Expand Up @@ -287,49 +266,19 @@ func TestEvaluatorWithPrepErrors(t *testing.T) {
// Create a logger for testing
handler := slog.NewTextHandler(os.Stdout, nil)

// Test with StaticProvider (which doesn't support adding data)
staticProvider := data.NewStaticProvider(map[string]any{"static": "data"})
evaluator, err := polyscript.FromRisorString(`ctx["static"]`,
options.WithLogHandler(handler),
options.WithDataProvider(staticProvider),
risorCompiler.WithGlobals([]string{constants.Ctx}),
// Test with StaticProvider (only testing specific error cases)
staticData := map[string]any{"static": "data"}
evaluator, err := polyscript.FromRisorStringWithData(`ctx["static"]`,
staticData,
handler,
)
require.NoError(t, err)

// Try to prepare context with StaticProvider
// The context should still be usable
ctx := context.Background()
_, err = evaluator.PrepareContext(ctx, map[string]any{"greeting": "Hello"})

// Should return error about StaticProvider not supporting runtime data changes
assert.Error(t, err, "Should return error for static provider")
assert.Contains(
t,
err.Error(),
"StaticProvider doesn't support adding data",
"Error should mention static provider limitation",
)

// Test with evaluator that has a ContextProvider
contextProvider := data.NewContextProvider(constants.EvalData)
evaluator, err = polyscript.FromRisorString(`ctx["request"]["ID"] || "no id"`,
options.WithLogHandler(handler),
options.WithDataProvider(contextProvider),
risorCompiler.WithGlobals([]string{constants.Ctx}),
)
require.NoError(t, err)

// Try to prepare context with unsupported data
enrichedCtx, err := evaluator.PrepareContext(ctx, 123) // Integer not supported directly

// Should return error about unsupported data type, but still return the context
assert.Error(t, err, "Should return error for unsupported data type")
assert.Contains(
t,
err.Error(),
"unsupported data type",
"Error should mention unsupported data type",
)

// The context should still be usable
assert.NotNil(t, enrichedCtx, "Should still return a context even with error")
// We expect an error because integers aren't directly supported
assert.Error(t, err, "PrepareContext should return an error for integers")
assert.NotNil(t, enrichedCtx, "Should return a context regardless")
}
44 changes: 0 additions & 44 deletions engine/options/defaults.go
View file
Open in desktop

This file was deleted.

Loading