From b3d503683337510c0f6b9a07bb3daf426ea694f3 Mon Sep 17 00:00:00 2001 From: Sebastian Tiedtke Date: Mon, 30 Sep 2024 16:23:37 -0700 Subject: [PATCH 1/6] Make no lifecycle identity the default --- pkg/document/document_test.go | 44 +++++++++++++++++++++---------- pkg/document/identity/resolver.go | 2 +- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 0ff913007..08a60c4aa 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -11,7 +11,7 @@ import ( "github.com/stateful/runme/v3/pkg/document/identity" ) -var testIdentityResolver = identity.NewResolver(identity.DefaultLifecycleIdentity) +var allIdentityResolver = identity.NewResolver(identity.AllLifecycleIdentity) func TestDocument_Parse(t *testing.T) { data := []byte(`# Examples @@ -36,7 +36,7 @@ First paragraph. 2. Item 2 3. Item 3 `) - doc := New(data, testIdentityResolver) + doc := New(data, allIdentityResolver) node, err := doc.Root() require.NoError(t, err) assert.Len(t, node.children, 4) @@ -70,17 +70,33 @@ First paragraph. } func TestDocument_Frontmatter(t *testing.T) { - t.Run("Parse", func(t *testing.T) { - data := bytes.TrimSpace([]byte(` + lcidRegex := `---\nkey: value\nrunme:\n id: .*\n version: v(?:[3-9]\d*|2\.\d+\.\d+|2\.\d+|\d+)\n---` + data := bytes.TrimSpace([]byte(` --- key: value --- First paragraph `, - )) + )) + + t.Run("Parse_DefaultIdentity", func(t *testing.T) { + var defaultIdentityResolver = identity.NewResolver(identity.DefaultLifecycleIdentity) + doc := New(data, defaultIdentityResolver) + err := doc.Parse() + require.NoError(t, err) + assert.Equal(t, []byte("First paragraph"), doc.Content()) + assert.Equal(t, 20, doc.ContentOffset()) + + frontmatter, err := doc.FrontmatterWithError() + require.NoError(t, err) + marshaledFrontmatter, err := frontmatter.Marshal(defaultIdentityResolver.DocumentEnabled()) + require.NoError(t, err) + assert.NotRegexp(t, lcidRegex, string(marshaledFrontmatter)) + }) - doc := New(data, testIdentityResolver) + t.Run("Parse_AllIdentity", func(t *testing.T) { + doc := New(data, allIdentityResolver) err := doc.Parse() require.NoError(t, err) assert.Equal(t, []byte("First paragraph"), doc.Content()) @@ -88,9 +104,9 @@ First paragraph frontmatter, err := doc.FrontmatterWithError() require.NoError(t, err) - marshaledFrontmatter, err := frontmatter.Marshal(testIdentityResolver.DocumentEnabled()) + marshaledFrontmatter, err := frontmatter.Marshal(allIdentityResolver.DocumentEnabled()) require.NoError(t, err) - assert.Regexp(t, `---\nkey: value\nrunme:\n id: .*\n version: v(?:[3-9]\d*|2\.\d+\.\d+|2\.\d+|\d+)\n---`, string(marshaledFrontmatter)) + assert.Regexp(t, lcidRegex, string(marshaledFrontmatter)) }) t.Run("Format", func(t *testing.T) { @@ -137,7 +153,7 @@ shell = "fish" for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - doc := New(tc.Data, testIdentityResolver) + doc := New(tc.Data, allIdentityResolver) _, err := doc.Root() require.NoError(t, err) @@ -158,7 +174,7 @@ shell = "fish" --- `, )) - doc := New(data, testIdentityResolver) + doc := New(data, allIdentityResolver) err := doc.Parse() require.NoError(t, err) @@ -172,7 +188,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) { data := []byte(`This will test final line breaks`) t.Run("No breaks", func(t *testing.T) { - doc := New(data, testIdentityResolver) + doc := New(data, allIdentityResolver) astNode, err := doc.RootAST() require.NoError(t, err) @@ -189,7 +205,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) { t.Run("1 LF", func(t *testing.T) { withLineBreaks := append(data, bytes.Repeat([]byte{'\n'}, 1)...) - doc := New(withLineBreaks, testIdentityResolver) + doc := New(withLineBreaks, allIdentityResolver) astNode, err := doc.RootAST() require.NoError(t, err) @@ -206,7 +222,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) { t.Run("1 CRLF", func(t *testing.T) { withLineBreaks := append(data, bytes.Repeat([]byte{'\r', '\n'}, 1)...) - doc := New(withLineBreaks, testIdentityResolver) + doc := New(withLineBreaks, allIdentityResolver) astNode, err := doc.RootAST() require.NoError(t, err) @@ -223,7 +239,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) { t.Run("7 LFs", func(t *testing.T) { withLineBreaks := append(data, bytes.Repeat([]byte{'\n'}, 7)...) - doc := New(withLineBreaks, testIdentityResolver) + doc := New(withLineBreaks, allIdentityResolver) astNode, err := doc.RootAST() require.NoError(t, err) diff --git a/pkg/document/identity/resolver.go b/pkg/document/identity/resolver.go index 11cb514c7..708396a5a 100644 --- a/pkg/document/identity/resolver.go +++ b/pkg/document/identity/resolver.go @@ -23,7 +23,7 @@ const ( CellLifecycleIdentity ) -const DefaultLifecycleIdentity = AllLifecycleIdentity +const DefaultLifecycleIdentity = UnspecifiedLifecycleIdentity var documentIdentities = &LifecycleIdentities{ AllLifecycleIdentity, From 266ef53288fd52d457c187d7c99c8d4d22a911ef Mon Sep 17 00:00:00 2001 From: Sebastian Tiedtke Date: Tue, 1 Oct 2024 08:38:43 -0700 Subject: [PATCH 2/6] Decide lifecycle identity persistence exclusively in parser --- internal/cmd/fmt.go | 25 ++++++-- internal/telemetry/scarf.go | 6 +- pkg/document/editor/cell.go | 3 +- pkg/document/editor/editor.go | 31 ++++++++++ pkg/document/editor/editor_test.go | 30 ++++++++++ .../editor/editorservice/service_test.go | 43 ++++--------- pkg/document/identity/resolver.go | 4 -- pkg/project/formatter.go | 60 +++++++++---------- 8 files changed, 128 insertions(+), 74 deletions(-) diff --git a/internal/cmd/fmt.go b/internal/cmd/fmt.go index c97fdebd3..6a7e27871 100644 --- a/internal/cmd/fmt.go +++ b/internal/cmd/fmt.go @@ -2,17 +2,20 @@ package cmd import ( "fmt" + "strings" "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/stateful/runme/v3/pkg/document/identity" "github.com/stateful/runme/v3/pkg/project" ) func fmtCmd() *cobra.Command { var ( - formatJSON bool - flatten bool - write bool + flatten bool + formatJSON bool + identityStr string + write bool ) cmd := cobra.Command{ @@ -39,7 +42,19 @@ func fmtCmd() *cobra.Command { } } - return project.FormatFiles(files, flatten, formatJSON, write, func(file string, formatted []byte) error { + var identityResolver *identity.IdentityResolver + switch strings.ToLower(identityStr) { + case "all": + identityResolver = identity.NewResolver(identity.AllLifecycleIdentity) + case "doc", "document": + identityResolver = identity.NewResolver(identity.DocumentLifecycleIdentity) + case "cell": + identityResolver = identity.NewResolver(identity.CellLifecycleIdentity) + default: + identityResolver = identity.NewResolver(identity.DefaultLifecycleIdentity) + } + + return project.FormatFiles(files, identityResolver, formatJSON, write, func(file string, formatted []byte) error { out := cmd.OutOrStdout() _, _ = fmt.Fprintf(out, "===== %s =====\n", file) _, _ = out.Write(formatted) @@ -54,6 +69,8 @@ func fmtCmd() *cobra.Command { cmd.Flags().BoolVar(&flatten, "flatten", true, "Flatten nested blocks in the output. WARNING: This can currently break frontmatter if turned off.") cmd.Flags().BoolVar(&formatJSON, "json", false, "Print out data as JSON. Only possible with --flatten and not allowed with --write.") cmd.Flags().BoolVarP(&write, "write", "w", false, "Write result to the source file instead of stdout.") + cmd.Flags().StringVar(&identityStr, "identity", "", "Set the lifecycle identity. Overrides the default.") + _ = cmd.Flags().MarkDeprecated("flatten", "This flag is now default and no longer has any other effect.") return &cmd } diff --git a/internal/telemetry/scarf.go b/internal/telemetry/scarf.go index e833d370b..e046f7a13 100644 --- a/internal/telemetry/scarf.go +++ b/internal/telemetry/scarf.go @@ -19,8 +19,10 @@ const ( client = "Kernel" ) -type reporterFunc func() error -type lookupEnvFunc func(key string) (string, bool) +type ( + reporterFunc func() error + lookupEnvFunc func(key string) (string, bool) +) var reporter reporterFunc diff --git a/pkg/document/editor/cell.go b/pkg/document/editor/cell.go index 6ac8cde7d..f9b527663 100644 --- a/pkg/document/editor/cell.go +++ b/pkg/document/editor/cell.go @@ -163,8 +163,7 @@ func toCellsRec( // In the future, we will include language detection (#77). metadata := block.Attributes() - cellID := block.ID() - if cellID != "" { + if cellID := block.ID(); cellID != "" { metadata[PrefixAttributeName(InternalAttributePrefix, "id")] = cellID } metadata[PrefixAttributeName(InternalAttributePrefix, "name")] = block.Name() diff --git a/pkg/document/editor/editor.go b/pkg/document/editor/editor.go index ca9e6c4b6..bf35e9c15 100644 --- a/pkg/document/editor/editor.go +++ b/pkg/document/editor/editor.go @@ -16,6 +16,7 @@ import ( const ( FrontmatterKey = "frontmatter" DocumentID = "id" + CellID = "id" ) type Options struct { @@ -67,9 +68,39 @@ func Deserialize(data []byte, opts Options) (*Notebook, error) { notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, DocumentID)] = opts.IdentityResolver.EphemeralDocumentID() } + // Make ephemeral cell IDs permanent if the cell lifecycle ID is enabled. + if opts.IdentityResolver.CellEnabled() { + err := applyCellLifecycleIdentity(notebook) + if err != nil { + return nil, err + } + } + return notebook, nil } +func applyCellLifecycleIdentity(notebook *Notebook) error { + ephCellIDKey := PrefixAttributeName(InternalAttributePrefix, CellID) + for _, cell := range notebook.Cells { + if cell.Kind != CodeKind { + continue + } + + // don't overwrite existing cell ID + if _, ok := cell.Metadata["id"]; ok { + continue + } + + // make sure we have an ephemeral cell ID + if _, ok := cell.Metadata[ephCellIDKey]; !ok { + return errors.Errorf("missing ephemeral cell ID") + } + + cell.Metadata[CellID] = cell.Metadata[ephCellIDKey] + } + return nil +} + func Serialize(notebook *Notebook, outputMetadata *document.RunmeMetadata, opts Options) ([]byte, error) { var result []byte var err error diff --git a/pkg/document/editor/editor_test.go b/pkg/document/editor/editor_test.go index 318e24a31..d372f9319 100644 --- a/pkg/document/editor/editor_test.go +++ b/pkg/document/editor/editor_test.go @@ -20,6 +20,7 @@ import ( var ( identityResolverNone = identity.NewResolver(identity.UnspecifiedLifecycleIdentity) identityResolverAll = identity.NewResolver(identity.AllLifecycleIdentity) + identityResolverCell = identity.NewResolver(identity.CellLifecycleIdentity) testMockID = ulid.GenerateID() ) @@ -126,6 +127,35 @@ func TestEditor_CodeBlock(t *testing.T) { }) } +func TestEditor_CellLifecycleIdentity(t *testing.T) { + t.Run("NoIdentity", func(t *testing.T) { + notebook, err := Deserialize(testDataNested, Options{IdentityResolver: identityResolverNone}) + require.NoError(t, err) + + for _, cell := range notebook.Cells { + if cell.Kind != CellKind(document.CodeBlockKind) { + continue + } + assert.Empty(t, cell.Metadata["id"]) + assert.NotEmpty(t, cell.Metadata[PrefixAttributeName(InternalAttributePrefix, "id")]) + } + }) + + t.Run("CellIdentity", func(t *testing.T) { + notebook, err := Deserialize(testDataNested, Options{IdentityResolver: identityResolverCell}) + require.NoError(t, err) + + for _, cell := range notebook.Cells { + if cell.Kind != CellKind(document.CodeBlockKind) { + continue + } + assert.NotEmpty(t, cell.Metadata["id"]) + assert.NotEmpty(t, cell.Metadata[PrefixAttributeName(InternalAttributePrefix, "id")]) + assert.Equal(t, cell.Metadata["id"], cell.Metadata[PrefixAttributeName(InternalAttributePrefix, "id")]) + } + }) +} + func TestEditor_Metadata(t *testing.T) { data := []byte(`# Heading Level 1 Paragraph 1 with a link [Link1](https://example.com 'Link Title 1') and a second link [Link2](https://example2.com 'Link Title 2').. diff --git a/pkg/document/editor/editorservice/service_test.go b/pkg/document/editor/editorservice/service_test.go index b1389b0af..52f852f44 100644 --- a/pkg/document/editor/editorservice/service_test.go +++ b/pkg/document/editor/editorservice/service_test.go @@ -106,7 +106,7 @@ func Test_IdentityUnspecified(t *testing.T) { assert.Len(t, dResp.Notebook.Metadata, 2) } - sResp, err := serializeWithIdentityPersistence(client, dResp.Notebook, identity) + sResp, err := serializeWithoutOutputs(client, dResp.Notebook) assert.NoError(t, err) content := string(sResp.Result) @@ -148,7 +148,7 @@ func Test_IdentityAll(t *testing.T) { assert.Contains(t, rawFrontmatter, "id: "+testMockID) assert.Contains(t, rawFrontmatter, "version: "+version.BaseVersion()) - sResp, err := serializeWithIdentityPersistence(client, dResp.Notebook, identity) + sResp, err := serializeWithoutOutputs(client, dResp.Notebook) assert.NoError(t, err) content := string(sResp.Result) @@ -156,7 +156,7 @@ func Test_IdentityAll(t *testing.T) { assert.Contains(t, content, "runme:\n") assert.Contains(t, content, "id: "+testMockID) assert.Contains(t, content, "version: "+version.BaseVersion()) - assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"foo\"}\n") + assert.Contains(t, content, "```sh {\"id\":\"123\",\"name\":\"foo\"}\n") assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"bar\"}\n") assert.Contains(t, content, "```js {\"id\":\""+testMockID+"\"}\n") } @@ -189,7 +189,7 @@ func Test_IdentityDocument(t *testing.T) { assert.Contains(t, rawFrontmatter, "id: "+testMockID) assert.Regexpf(t, versionRegex, rawFrontmatter, "Wrong version") - sResp, err := serializeWithIdentityPersistence(client, dResp.Notebook, identity) + sResp, err := serializeWithoutOutputs(client, dResp.Notebook) assert.NoError(t, err) content := string(sResp.Result) @@ -231,7 +231,7 @@ func Test_IdentityCell(t *testing.T) { assert.Len(t, dResp.Notebook.Metadata, 2) } - sResp, err := serializeWithIdentityPersistence(client, dResp.Notebook, identity) + sResp, err := serializeWithoutOutputs(client, dResp.Notebook) assert.NoError(t, err) content := string(sResp.Result) @@ -245,7 +245,7 @@ func Test_IdentityCell(t *testing.T) { assert.NotRegexp(t, "^\n\n", content) } - assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"foo\"}\n") + assert.Contains(t, content, "```sh {\"id\":\"123\",\"name\":\"foo\"}\n") assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"bar\"}\n") assert.Contains(t, content, "```js {\"id\":\""+testMockID+"\"}\n") } @@ -273,7 +273,7 @@ func Test_RunmelessFrontmatter(t *testing.T) { assert.NotContains(t, rawFrontmatter, "id: \"123\"\n") assert.NotRegexp(t, versionRegex, rawFrontmatter, "Wrong version") - sResp, err := serializeWithIdentityPersistence(client, dResp.Notebook, identity) + sResp, err := serializeWithoutOutputs(client, dResp.Notebook) assert.NoError(t, err) content := string(sResp.Result) @@ -285,7 +285,7 @@ func Test_RunmelessFrontmatter(t *testing.T) { assert.Regexp(t, "^---\n", content) assert.NotRegexp(t, "^\n\n", content) - assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"foo\"}\n") + assert.Contains(t, content, "```sh {\"id\":\"123\",\"name\":\"foo\"}\n") assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"bar\"}\n") assert.Contains(t, content, "```js {\"id\":\""+testMockID+"\"}\n") } @@ -312,7 +312,7 @@ func Test_RetainInvalidFrontmatter(t *testing.T) { assert.NotContains(t, rawFrontmatter, "id: ") assert.NotRegexp(t, versionRegex, rawFrontmatter, "Wrong version") - sResp, err := serializeWithIdentityPersistence(client, dResp.Notebook, identity) + sResp, err := serializeWithoutOutputs(client, dResp.Notebook) assert.NoError(t, err) content := string(sResp.Result) @@ -324,7 +324,7 @@ func Test_RetainInvalidFrontmatter(t *testing.T) { assert.Regexp(t, "^\\+\\+\\+\n", content) assert.NotRegexp(t, "^\n\n", content) - assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"foo\"}\n") + assert.Contains(t, content, "```sh {\"id\":\"123\",\"name\":\"foo\"}\n") assert.Contains(t, content, "```sh {\"id\":\""+testMockID+"\",\"name\":\"bar\"}\n") assert.Contains(t, content, "```js {\"id\":\""+testMockID+"\"}\n") } @@ -447,14 +447,8 @@ func deserialize(client parserv1.ParserServiceClient, content string, idt parser ) } -func serializeWithIdentityPersistence(client parserv1.ParserServiceClient, notebook *parserv1.Notebook, idt parserv1.RunmeIdentity) (*parserv1.SerializeResponse, error) { - persistIdentityLikeExtension(notebook) - return client.Serialize( - context.Background(), - &parserv1.SerializeRequest{ - Notebook: notebook, - }, - ) +func serializeWithoutOutputs(client parserv1.ParserServiceClient, notebook *parserv1.Notebook) (*parserv1.SerializeResponse, error) { + return serializeWithOutputs(client, notebook, &parserv1.SerializeRequestOptions{}) } func serializeWithOutputs(client parserv1.ParserServiceClient, notebook *parserv1.Notebook, options *parserv1.SerializeRequestOptions) (*parserv1.SerializeResponse, error) { @@ -466,16 +460,3 @@ func serializeWithOutputs(client parserv1.ParserServiceClient, notebook *parserv }, ) } - -// mimics what would happen on the extension side -func persistIdentityLikeExtension(notebook *parserv1.Notebook) { - for _, cell := range notebook.Cells { - // todo(sebastian): preserve original id when they are set? - // if _, ok := cell.Metadata["id"]; ok { - // break - // } - if v, ok := cell.Metadata["runme.dev/id"]; ok { - cell.Metadata["id"] = v - } - } -} diff --git a/pkg/document/identity/resolver.go b/pkg/document/identity/resolver.go index 708396a5a..51ad3c8e5 100644 --- a/pkg/document/identity/resolver.go +++ b/pkg/document/identity/resolver.go @@ -78,10 +78,6 @@ func (ir *IdentityResolver) EphemeralDocumentID() string { // GetCellID returns a cell ID and a boolean indicating if it's new or from attributes. func (ir *IdentityResolver) GetCellID(obj any, attributes map[string]string) (string, bool) { - if !ir.cellIdentity { - return "", false - } - // todo(sebastian): are invalid ulid's valid IDs? // Check for a valid 'id' in attributes; // if present and valid due to explicit cell identity cache and return it. diff --git a/pkg/project/formatter.go b/pkg/project/formatter.go index 08cbd3b1a..6cb60bb8b 100644 --- a/pkg/project/formatter.go +++ b/pkg/project/formatter.go @@ -5,22 +5,20 @@ import ( "encoding/json" "github.com/pkg/errors" - "github.com/stateful/runme/v3/internal/renderer/cmark" - "github.com/stateful/runme/v3/pkg/document" "github.com/stateful/runme/v3/pkg/document/editor" "github.com/stateful/runme/v3/pkg/document/identity" ) type FuncOutput func(string, []byte) error -func FormatFiles(files []string, flatten bool, formatJSON bool, write bool, outputter FuncOutput) error { +func FormatFiles(files []string, identityResolver *identity.IdentityResolver, formatJSON bool, write bool, outputter FuncOutput) error { for _, file := range files { data, err := readMarkdown(file) if err != nil { return err } - formatted, err := formatFile(data, flatten, formatJSON) + formatted, err := formatFile(data, identityResolver, formatJSON) if err != nil { return err } @@ -38,41 +36,41 @@ func FormatFiles(files []string, flatten bool, formatJSON bool, write bool, outp return nil } -func formatFile(data []byte, flatten bool, formatJSON bool) ([]byte, error) { - identityResolver := identity.NewResolver(identity.DefaultLifecycleIdentity) +func formatFile(data []byte, identityResolver *identity.IdentityResolver, formatJSON bool) ([]byte, error) { var formatted []byte - if flatten { - notebook, err := editor.Deserialize(data, editor.Options{IdentityResolver: identityResolver}) - if err != nil { - return nil, errors.Wrap(err, "failed to deserialize") - } + notebook, err := editor.Deserialize(data, editor.Options{IdentityResolver: identityResolver}) + if err != nil { + return nil, errors.Wrap(err, "failed to deserialize") + } - if formatJSON { - var buf bytes.Buffer - enc := json.NewEncoder(&buf) - enc.SetIndent("", " ") - if err := enc.Encode(notebook); err != nil { - return nil, errors.Wrap(err, "failed to encode to JSON") - } - formatted = buf.Bytes() - } else { - formatted, err = editor.Serialize(notebook, nil, editor.Options{IdentityResolver: identityResolver}) - if err != nil { - return nil, errors.Wrap(err, "failed to serialize") - } + if formatJSON { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + if err := enc.Encode(notebook); err != nil { + return nil, errors.Wrap(err, "failed to encode to JSON") } + formatted = buf.Bytes() } else { - doc := document.New(data, identityResolver) - astNode, err := doc.RootAST() + formatted, err = editor.Serialize(notebook, nil, editor.Options{IdentityResolver: identityResolver}) if err != nil { - return nil, errors.Wrap(err, "failed to parse source") - } - formatted, err = cmark.Render(astNode, data) - if err != nil { - return nil, errors.Wrap(err, "failed to render") + return nil, errors.Wrap(err, "failed to serialize") } } + // todo(sebastian): remove moving to beta? it's neither used nor maintained + // { + // doc := document.New(data, identityResolver) + // astNode, err := doc.RootAST() + // if err != nil { + // return nil, errors.Wrap(err, "failed to parse source") + // } + // formatted, err = cmark.Render(astNode, data) + // if err != nil { + // return nil, errors.Wrap(err, "failed to render") + // } + // } + return formatted, nil } From 106f8f1b4fdfb4ea0644a4b3debb3951aef77c02 Mon Sep 17 00:00:00 2001 From: Sebastian Tiedtke Date: Tue, 1 Oct 2024 10:49:13 -0700 Subject: [PATCH 3/6] Fix flakey tests, now independent of winsize and exact char sequences --- internal/command/command_terminal.go | 13 +++++++---- internal/command/command_terminal_test.go | 28 +++++++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/internal/command/command_terminal.go b/internal/command/command_terminal.go index 786ca3ab3..a301f2f6a 100644 --- a/internal/command/command_terminal.go +++ b/internal/command/command_terminal.go @@ -9,9 +9,10 @@ import ( "go.uber.org/zap" ) -var introMsg = []byte( - " # Runme terminal: Upon exit, exported environment variables will roll up into your session." + - " Type 'save' to add this session to the notebook.\n\n", +const ( + introFirstLine = "Runme terminal: Upon exit, exported environment variables will roll up into your session." + introSecondLine = "Type 'save' to add this session to the notebook." + envSourceCmd = "runme beta env source --silent --insecure --export" ) type terminalCommand struct { @@ -53,12 +54,14 @@ func (c *terminalCommand) Start(ctx context.Context) (err error) { } } - if _, err := c.stdinWriter.Write([]byte(" eval $(runme beta env source --silent --insecure --export)\n alias save=\"exit\"\n clear\n")); err != nil { + if _, err := c.stdinWriter.Write([]byte(" eval $(" + envSourceCmd + ")\n alias save=\"exit\"\n clear\n")); err != nil { return err } // todo(sebastian): good enough for prototype; it makes more sense to write this message at the TTY-level - _, err = c.stdinWriter.Write(introMsg) + _, err = c.stdinWriter.Write([]byte( + " # " + introFirstLine + + " " + introSecondLine + "\n\n")) return err } diff --git a/internal/command/command_terminal_test.go b/internal/command/command_terminal_test.go index 3b0412b13..2c9525d59 100644 --- a/internal/command/command_terminal_test.go +++ b/internal/command/command_terminal_test.go @@ -49,12 +49,12 @@ func TestTerminalCommand_EnvPropagation(t *testing.T) { // Terminal command sets up a trap on EXIT. // Wait for it before starting to send commands. - expectContainLine(t, stdout, "trap -- \"__cleanup\" EXIT") + expectContainLines(t, stdout, []string{"trap -- \"__cleanup\" EXIT"}) _, err = stdinW.Write([]byte("export TEST_ENV=1\n")) require.NoError(t, err) // Wait for the prompt before sending the next command. - expectContainLine(t, stdout, "$") + expectContainLines(t, stdout, []string{"$"}) _, err = stdinW.Write([]byte("exit\n")) require.NoError(t, err) @@ -91,23 +91,31 @@ func TestTerminalCommand_Intro(t *testing.T) { require.NoError(t, cmd.Start(ctx)) - expectContainLine(t, stdout, "eval $(runme beta env source --silent --insecure --export)") - // todo(sebastian): why won't this work on docker? winsize? - // expectContainLine(t, stdout, strings.Trim(string(introMsg), " \r\n")) + expectContainLines(t, stdout, []string{envSourceCmd, introSecondLine}) } -func expectContainLine(t *testing.T, r io.Reader, expected string) { +func expectContainLines(t *testing.T, r io.Reader, expected []string) { t.Helper() + var output strings.Builder + hits := make(map[string]bool, len(expected)) for { scanner := bufio.NewScanner(r) for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, expected) { - return - } + _, _ = output.WriteString(scanner.Text()) } require.NoError(t, scanner.Err()) + + for _, e := range expected { + if strings.Contains(output.String(), e) { + hits[e] = true + } + } + + if len(hits) == len(expected) { + return + } + time.Sleep(time.Millisecond * 400) } } From a691aafdd9da5a1a36cec1851cc37f433e440474 Mon Sep 17 00:00:00 2001 From: Sebastian Tiedtke Date: Tue, 1 Oct 2024 11:02:49 -0700 Subject: [PATCH 4/6] Lint ignores and F --- cmd/gqltool/main.go | 6 ++---- internal/cmd/code_server.go | 3 +++ internal/cmd/code_server_test.go | 2 +- internal/runner/service.go | 4 +--- internal/runnerv2service/service_resolve_program.go | 4 +--- pkg/document/document_test.go | 2 +- 6 files changed, 9 insertions(+), 12 deletions(-) diff --git a/cmd/gqltool/main.go b/cmd/gqltool/main.go index 41b03fe95..29479b83f 100644 --- a/cmd/gqltool/main.go +++ b/cmd/gqltool/main.go @@ -11,10 +11,8 @@ import ( "github.com/stateful/runme/v3/internal/client/graphql" ) -var ( - apiURL = flag.String("api-url", "http://localhost:4000", "The API base address") - // tokenDir = flag.String("token-dir", cmd.GetUserConfigHome(), "The directory with tokens") -) +// var tokenDir = flag.String("token-dir", cmd.GetUserConfigHome(), "The directory with tokens") +var apiURL = flag.String("api-url", "http://localhost:4000", "The API base address") func init() { flag.Parse() diff --git a/internal/cmd/code_server.go b/internal/cmd/code_server.go index caefda527..522d90f90 100644 --- a/internal/cmd/code_server.go +++ b/internal/cmd/code_server.go @@ -239,6 +239,7 @@ func runCodeServerCommand(cmd *cobra.Command, execFile string, routeToBuffer boo return buffer.Bytes(), nil } +//lint:ignore U1000 Ignore because it's only used in tests running in docker func getLatestExtensionVersion(experimental bool) (string, error) { var tagName string @@ -286,6 +287,7 @@ func getLatestExtensionVersion(experimental bool) (string, error) { return tagName, nil } +//lint:ignore U1000 Ignore because it's only used in tests running in docker func getExtensionURL(tagName string) (string, error) { var arch string @@ -322,6 +324,7 @@ func getExtensionURL(tagName string) (string, error) { return downloadURL, nil } +//lint:ignore U1000 Ignore because it's only used in tests running in docker func downloadVscodeExtension(dir string, experimental bool) (string, error) { tagName, err := getLatestExtensionVersion(experimental) if err != nil { diff --git a/internal/cmd/code_server_test.go b/internal/cmd/code_server_test.go index 97019b198..d98c614b2 100644 --- a/internal/cmd/code_server_test.go +++ b/internal/cmd/code_server_test.go @@ -1,4 +1,4 @@ -//go:test !darwin +//go:build test_with_docker package cmd diff --git a/internal/runner/service.go b/internal/runner/service.go index 6786a08e0..a194f610d 100644 --- a/internal/runner/service.go +++ b/internal/runner/service.go @@ -682,9 +682,7 @@ func (r *runnerService) ResolveProgram(ctx context.Context, req *runnerv1.Resolv return nil, err } - var ( - modifiedScriptBuf bytes.Buffer - ) + var modifiedScriptBuf bytes.Buffer script := req.GetScript() if commands := req.GetCommands(); script == "" && len(commands.Lines) > 0 { diff --git a/internal/runnerv2service/service_resolve_program.go b/internal/runnerv2service/service_resolve_program.go index 987370427..fe4d3bcff 100644 --- a/internal/runnerv2service/service_resolve_program.go +++ b/internal/runnerv2service/service_resolve_program.go @@ -26,9 +26,7 @@ func (r *runnerService) ResolveProgram(ctx context.Context, req *runnerv2.Resolv return nil, err } - var ( - modifiedScriptBuf bytes.Buffer - ) + var modifiedScriptBuf bytes.Buffer script := req.GetScript() if commands := req.GetCommands(); script == "" && len(commands.Lines) > 0 { diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 08a60c4aa..065ed32f3 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -81,7 +81,7 @@ First paragraph )) t.Run("Parse_DefaultIdentity", func(t *testing.T) { - var defaultIdentityResolver = identity.NewResolver(identity.DefaultLifecycleIdentity) + defaultIdentityResolver := identity.NewResolver(identity.DefaultLifecycleIdentity) doc := New(data, defaultIdentityResolver) err := doc.Parse() require.NoError(t, err) From ec948e6cdbbd74eb67da4ad82d2cfeb73ef79dd4 Mon Sep 17 00:00:00 2001 From: Sebastian Tiedtke Date: Wed, 2 Oct 2024 16:43:51 -0700 Subject: [PATCH 5/6] Clarify name cacheID since it's exclusively for caching --- pkg/document/editor/editor.go | 9 +++------ pkg/document/editor/editorservice/service_test.go | 6 +++--- pkg/document/identity/resolver.go | 4 ++-- pkg/document/identity/resolver_test.go | 4 ++-- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/document/editor/editor.go b/pkg/document/editor/editor.go index bf35e9c15..aabd393d3 100644 --- a/pkg/document/editor/editor.go +++ b/pkg/document/editor/editor.go @@ -15,7 +15,7 @@ import ( const ( FrontmatterKey = "frontmatter" - DocumentID = "id" + CacheID = "cacheId" CellID = "id" ) @@ -50,6 +50,8 @@ func Deserialize(data []byte, opts Options) (*Notebook, error) { Frontmatter: frontmatter, Metadata: map[string]string{ PrefixAttributeName(InternalAttributePrefix, constants.FinalLineBreaksKey): strconv.Itoa(doc.TrailingLineBreaksCount()), + // CacheID used for uniquely identifying documents in caches across vscode deserialization (issues) and serialization (retains). + PrefixAttributeName(InternalAttributePrefix, CacheID): opts.IdentityResolver.CacheID(), }, } @@ -63,11 +65,6 @@ func Deserialize(data []byte, opts Options) (*Notebook, error) { notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, FrontmatterKey)] = string(raw) } - // Store internal ephemeral document ID if the document lifecycle ID is disabled. - if !opts.IdentityResolver.DocumentEnabled() { - notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, DocumentID)] = opts.IdentityResolver.EphemeralDocumentID() - } - // Make ephemeral cell IDs permanent if the cell lifecycle ID is enabled. if opts.IdentityResolver.CellEnabled() { err := applyCellLifecycleIdentity(notebook) diff --git a/pkg/document/editor/editorservice/service_test.go b/pkg/document/editor/editorservice/service_test.go index 52f852f44..e697d9792 100644 --- a/pkg/document/editor/editorservice/service_test.go +++ b/pkg/document/editor/editorservice/service_test.go @@ -139,7 +139,7 @@ func Test_IdentityAll(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 2) + assert.Len(t, dResp.Notebook.Metadata, 3) if tt.hasExtraFrontmatter { assert.Contains(t, rawFrontmatter, "prop: value") @@ -180,7 +180,7 @@ func Test_IdentityDocument(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 2) + assert.Len(t, dResp.Notebook.Metadata, 3) if tt.hasExtraFrontmatter { assert.Contains(t, rawFrontmatter, "prop: value") @@ -307,7 +307,7 @@ func Test_RetainInvalidFrontmatter(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 2) + assert.Len(t, dResp.Notebook.Metadata, 3) assert.Contains(t, rawFrontmatter, "invalid frontmatter") assert.NotContains(t, rawFrontmatter, "id: ") assert.NotRegexp(t, versionRegex, rawFrontmatter, "Wrong version") diff --git a/pkg/document/identity/resolver.go b/pkg/document/identity/resolver.go index 51ad3c8e5..9a7df1ff3 100644 --- a/pkg/document/identity/resolver.go +++ b/pkg/document/identity/resolver.go @@ -71,8 +71,8 @@ func (ir *IdentityResolver) DocumentEnabled() bool { return ir.documentIdentity } -// EphemeralDocumentID returns a new document ID which is not persisted. -func (ir *IdentityResolver) EphemeralDocumentID() string { +// CacheID returns a new document ID which is not persisted. +func (ir *IdentityResolver) CacheID() string { return ulid.GenerateID() } diff --git a/pkg/document/identity/resolver_test.go b/pkg/document/identity/resolver_test.go index d74c651ef..705cb8c29 100644 --- a/pkg/document/identity/resolver_test.go +++ b/pkg/document/identity/resolver_test.go @@ -58,8 +58,8 @@ func TestIdentityResolver(t *testing.T) { assert.NotEmpty(t, id) }) - t.Run("EphemeralDocumentID", func(t *testing.T) { + t.Run("CacheID", func(t *testing.T) { resolver := NewResolver(DefaultLifecycleIdentity) - assert.Len(t, resolver.EphemeralDocumentID(), 26) + assert.Len(t, resolver.CacheID(), 26) }) } From a8c369ad119d9251975def321f22ca253219a504 Mon Sep 17 00:00:00 2001 From: Sebastian Tiedtke Date: Thu, 3 Oct 2024 12:22:22 -0700 Subject: [PATCH 6/6] Enable smooth transition --- pkg/document/editor/editor.go | 6 +++++- .../editor/editorservice/service_test.go | 16 ++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/document/editor/editor.go b/pkg/document/editor/editor.go index aabd393d3..7a1d04091 100644 --- a/pkg/document/editor/editor.go +++ b/pkg/document/editor/editor.go @@ -45,13 +45,14 @@ func Deserialize(data []byte, opts Options) (*Notebook, error) { opts.Logger().Warn("failed to parse frontmatter", zap.Error(fmErr)) } + cacheID := opts.IdentityResolver.CacheID() notebook := &Notebook{ Cells: toCells(doc, node, doc.Content()), Frontmatter: frontmatter, Metadata: map[string]string{ PrefixAttributeName(InternalAttributePrefix, constants.FinalLineBreaksKey): strconv.Itoa(doc.TrailingLineBreaksCount()), // CacheID used for uniquely identifying documents in caches across vscode deserialization (issues) and serialization (retains). - PrefixAttributeName(InternalAttributePrefix, CacheID): opts.IdentityResolver.CacheID(), + PrefixAttributeName(InternalAttributePrefix, CacheID): cacheID, }, } @@ -65,6 +66,9 @@ func Deserialize(data []byte, opts Options) (*Notebook, error) { notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, FrontmatterKey)] = string(raw) } + // todo(sebastian): faux document-level `runme.dev/id` to not break existing clients; revert in near future + notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, "id")] = cacheID + // Make ephemeral cell IDs permanent if the cell lifecycle ID is enabled. if opts.IdentityResolver.CellEnabled() { err := applyCellLifecycleIdentity(notebook) diff --git a/pkg/document/editor/editorservice/service_test.go b/pkg/document/editor/editorservice/service_test.go index e697d9792..74233d9ac 100644 --- a/pkg/document/editor/editorservice/service_test.go +++ b/pkg/document/editor/editorservice/service_test.go @@ -97,13 +97,13 @@ func Test_IdentityUnspecified(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] if tt.hasExtraFrontmatter { assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 3) + assert.Len(t, dResp.Notebook.Metadata, 4) assert.Contains(t, rawFrontmatter, "prop: value\n") assert.Contains(t, rawFrontmatter, "id: \"123\"\n") assert.Contains(t, rawFrontmatter, "version: v") } else { assert.False(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 2) + assert.Len(t, dResp.Notebook.Metadata, 3) } sResp, err := serializeWithoutOutputs(client, dResp.Notebook) @@ -139,7 +139,7 @@ func Test_IdentityAll(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 3) + assert.Len(t, dResp.Notebook.Metadata, 4) if tt.hasExtraFrontmatter { assert.Contains(t, rawFrontmatter, "prop: value") @@ -180,7 +180,7 @@ func Test_IdentityDocument(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 3) + assert.Len(t, dResp.Notebook.Metadata, 4) if tt.hasExtraFrontmatter { assert.Contains(t, rawFrontmatter, "prop: value") @@ -222,13 +222,13 @@ func Test_IdentityCell(t *testing.T) { if tt.hasExtraFrontmatter { assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 3) + assert.Len(t, dResp.Notebook.Metadata, 4) assert.Contains(t, rawFrontmatter, "prop: value\n") assert.Contains(t, rawFrontmatter, "id: \"123\"\n") assert.Regexp(t, versionRegex, rawFrontmatter, "Wrong version") } else { assert.False(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 2) + assert.Len(t, dResp.Notebook.Metadata, 3) } sResp, err := serializeWithoutOutputs(client, dResp.Notebook) @@ -268,7 +268,7 @@ func Test_RunmelessFrontmatter(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 3) + assert.Len(t, dResp.Notebook.Metadata, 4) assert.Contains(t, rawFrontmatter, "prop: value\n") assert.NotContains(t, rawFrontmatter, "id: \"123\"\n") assert.NotRegexp(t, versionRegex, rawFrontmatter, "Wrong version") @@ -307,7 +307,7 @@ func Test_RetainInvalidFrontmatter(t *testing.T) { rawFrontmatter, ok := dResp.Notebook.Metadata["runme.dev/frontmatter"] assert.True(t, ok) - assert.Len(t, dResp.Notebook.Metadata, 3) + assert.Len(t, dResp.Notebook.Metadata, 4) assert.Contains(t, rawFrontmatter, "invalid frontmatter") assert.NotContains(t, rawFrontmatter, "id: ") assert.NotRegexp(t, versionRegex, rawFrontmatter, "Wrong version")