Skip to content

Commit

Permalink
Refactor the gen command to use the new cli generator (#489)
Browse files Browse the repository at this point in the history
* Removed multiple files from pkg/gen/input and pkg/gen/output

The commit includes the deletion of several files in the 'pkg/gen/input' and 'pkg/gen/output' directories. This is part of a larger refactoring effort to streamline codebase, improve maintainability, and remove redundant or unnecessary code.

* Updated test_all dependencies in Makefile

The 'test_all' target in the Makefile has been updated to include new dependencies. These additional binaries are now required for the 'test_all' target to run successfully.

* Added test suite for uml2ts package

A new test suite has been added for the uml2ts package. This includes importing necessary packages and setting up a context, file system, and error handling. The tests are designed to check conformance of the 'interface' and 'nested_interface' with standard output. A generator description is also provided as part of the testing process.

* Refactor test assertion handling

The code has been refactored to improve the handling of test assertions. A panic is now thrown if a test contains no assertions, replacing the previous warning log. The error expectation in the generator execution has also been adjusted for better traceability. In addition, a warning is logged when there are no assertions for a specific test in the suite.

* Refactor end-to-end tests and improve error message

The end-to-end testing code has been refactored for better readability and maintainability. The changes include the use of a new function, `DescribeGenerator`, to simplify test generation. Additionally, the error message displayed when a file cannot be found has been made more descriptive. In the suite test file, the method for reading local TypeScript tests has also been updated to allow specific assertions per test type.

* Added stringer interface and logging to file operations

- Implemented fmt.Stringer interface in file, fsOutput, reader, and writerOutput structs for better debugging.
- Added debug logs for writing to a file, copying filesystems, and copying files in writerOutput.
- Counted the number of files copied in writerOutput for more detailed logging.

* Added logging and stdout support

Enhanced the parsing function with debug logs for better traceability. Added a new boolean field 'stdout' to the 'fromPath' struct in path.go, allowing plugins to expect standard output. Updated static.go to use this new feature by setting 'stdout' as true for the Uml2Ts plugin.

* Refactor gen command and error handling

Significant changes have been made to the 'gen' command. The code now uses a configuration object instead of parsing arguments directly. Error handling has been improved by adding more checks after each operation, ensuring that any issues are caught and reported immediately. Additionally, the input and output parsing logic has been updated to use new methods from the 'run' package. Finally, the generator execution process has been modified for better clarity and efficiency.

* Improved test reliability and refactored Docker image existence check

- Enhanced the robustness of tests by adding FlakeAttempts to uml2ts Conformance suite and using ExpectWithOffset in assertions.
- Refactored Docker plugin's image existence check into a separate function, ImageExists, for better readability and reusability.
- Updated docker plugin tests to use the new ImageExists function.

* Optimized string return in writerOutput

Removed unnecessary usage of fmt.Sprintf for returning a static string in the writerOutput function. This simplifies the code and potentially improves performance by avoiding unnecessary formatting operations.
  • Loading branch information
UnstoppableMango authored Nov 30, 2024
1 parent 7520daf commit f566251
Show file tree
Hide file tree
Showing 23 changed files with 169 additions and 259 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ clean:
bun run --cwd packages/tdl clean
bun run --cwd packages/ts clean

test_all:
test_all: bin/ux bin/uml2ts bin/uml2uml
$(GINKGO) run -r ./

e2e: .make/go_e2e_test
Expand Down
10 changes: 3 additions & 7 deletions cmd/ux/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@ var _ = Describe("End to end", func() {
Describe("TypeScript Conformance", FlakeAttempts(5), func() {
Describe("stdout", func() {
generator := cli.New("ux",
cli.WithArgs("gen", "ts"),
cli.WithArgs("gen", "ts", "-"),
cli.ExpectStdout,
)

for test := range typescriptSuite.Tests() {
conform.ItShouldPass(generator, test,
conform.AssertStdout,
)
}
conform.DescribeGenerator(typescriptSuite, generator)
})
})

Expand Down Expand Up @@ -51,7 +47,7 @@ var _ = Describe("End to end", func() {
out, err := cmd.CombinedOutput()

Expect(err).To(HaveOccurred())
Expect(string(out)).To(Equal("open fkjdslfkdjlsf: no such file or directory\n"))
Expect(string(out)).To(Equal("parsing run config: stat fkjdslfkdjlsf: no such file or directory\n"))
})

It("should write to output file", FlakeAttempts(5), func(ctx context.Context) {
Expand Down
5 changes: 4 additions & 1 deletion cmd/ux/ux_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ func TestUx(t *testing.T) {
g.Expect(os.Stat(bin)).NotTo(BeNil())

fs := afero.NewOsFs()
typescriptSuite, err = conform.ReadLocalTypeScriptSuite(ctx, fs)
typescriptSuite, err = conform.ReadLocalGitTests(ctx, fs, "typescript", map[string][]e2e.Assertion{
"interface": {conform.AssertStdout},
"nested_interface": {conform.AssertStdout},
})
g.Expect(err).NotTo(HaveOccurred())

RegisterFailHandler(Fail)
Expand Down
35 changes: 35 additions & 0 deletions packages/uml2ts/uml2ts_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package uml2ts_test

import (
"context"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/spf13/afero"
"github.com/unstoppablemango/tdl/pkg/conform"
"github.com/unstoppablemango/tdl/pkg/gen/cli"
"github.com/unstoppablemango/tdl/pkg/testing/e2e"
)

var suite e2e.Suite

func TestUml2ts(t *testing.T) {
g := NewWithT(t)
ctx := context.Background()
fs := afero.NewOsFs()

var err error
suite, err = conform.ReadLocalGitTests(ctx, fs, "typescript", map[string][]e2e.Assertion{
"interface": {conform.AssertStdout},
"nested_interface": {conform.AssertStdout},
})
g.Expect(err).NotTo(HaveOccurred())

RegisterFailHandler(Fail)
RunSpecs(t, "Uml2ts Suite")
}

var _ = Describe("uml2ts Conformance", FlakeAttempts(5), func() {
conform.DescribeGenerator(suite, cli.New("uml2ts", cli.ExpectStdout))
})
46 changes: 33 additions & 13 deletions pkg/cmd/gen.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package cmd

import (
"errors"

"github.com/charmbracelet/log"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/unstoppablemango/tdl/internal"
"github.com/unstoppablemango/tdl/internal/util"
"github.com/unstoppablemango/tdl/pkg/cmd/flags"
"github.com/unstoppablemango/tdl/pkg/gen/input"
"github.com/unstoppablemango/tdl/pkg/gen/output"
"github.com/unstoppablemango/tdl/pkg/config/run"
"github.com/unstoppablemango/tdl/pkg/plugin"
"github.com/unstoppablemango/tdl/pkg/spec"
"github.com/unstoppablemango/tdl/pkg/target"
Expand All @@ -22,11 +23,16 @@ func NewGen() *cobra.Command {
Args: cobra.RangeArgs(1, 3),
Run: func(cmd *cobra.Command, args []string) {
ctx := cmd.Context()
target, err := target.Parse(args[0])
config, err := run.ParseArgs(args)
if err != nil {
util.Fail(err)
}

log.Debug("parsing target")
target, err := target.Parse(config.Target)
if err != nil {
util.Fail(err)
}
log := log.With("target", target)

log.Debug("searching for a plugin")
plugin, err := plugin.FirstAvailable(target)
Expand All @@ -35,27 +41,41 @@ func NewGen() *cobra.Command {
}

log.Debug("searching for a generator")
generator, err := plugin.SinkGenerator(target)
generator, err := plugin.Generator(ctx, target)
if err != nil {
util.Fail(err)
}

fsys := afero.NewOsFs()
input, err := input.ParseArgs(fsys, args[1:])
log.Debug("parsing inputs")
os := internal.RealOs()
inputs, err := run.ParseInputs(os, config)
if err != nil {
util.Fail(err)
}
if len(inputs) != 1 {
util.Fail(errors.New("only a single input may be provided"))
}

output, err := output.ParseArgs(fsys, args[1:])
log.Debugf("reading spec: %s", inputs[0])
spec, err := spec.ReadInput(inputs[0])
if err != nil {
util.Fail(err)
}

log.Debug("creating pipeline")
pipeline := spec.PipeInput(generator.Execute)
log.Debugf("executing generator: %s", generator)
fs, err := generator.Execute(ctx, spec)
if err != nil {
util.Fail(err)
}

log.Debug("parsing output")
output, err := run.ParseOutput(os, config)
if err != nil {
util.Fail(err)
}

log.Debug("executing pipeline")
if err := pipeline(ctx, input, output); err != nil {
log.Debugf("writing output: %s %s", fs.Name(), output)
if err = output.Write(fs); err != nil {
util.Fail(err)
}
},
Expand Down
13 changes: 13 additions & 0 deletions pkg/config/run/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/fs"
"os"

"github.com/charmbracelet/log"
"github.com/spf13/afero"
tdl "github.com/unstoppablemango/tdl/pkg"
"github.com/unstoppablemango/tdl/pkg/mediatype"
Expand All @@ -16,6 +17,11 @@ type file struct {
media tdl.MediaType
}

// String implements tdl.Input.
func (f *file) String() string {
return fmt.Sprintf("file: %s %s", f.Name(), f.media)
}

func (f *file) MediaType() tdl.MediaType {
return f.media
}
Expand Down Expand Up @@ -47,6 +53,11 @@ type fsOutput struct {
path string
}

// String implements tdl.Output.
func (f *fsOutput) String() string {
return fmt.Sprintf("fs: %s %s", f.dest.Name(), f.path)
}

func (f *fsOutput) Write(output afero.Fs) error {
stat, err := f.dest.Stat(f.path)
if errors.Is(err, os.ErrNotExist) {
Expand All @@ -68,6 +79,7 @@ func (f *fsOutput) writeFile(output afero.Fs) error {
return err
}

log.Debugf("writing to file %s", file.Name())
return WriterOutput(file).Write(output)
}

Expand All @@ -76,6 +88,7 @@ func FsOutput(dest afero.Fs, path string) tdl.Output {
}

func copyFs(src, dest afero.Fs) error {
log.Debugf("copying fs %s to fs %s", src.Name(), dest.Name())
return afero.Walk(src, "",
func(path string, info fs.FileInfo, err error) error {
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions pkg/config/run/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"

"github.com/charmbracelet/log"
tdl "github.com/unstoppablemango/tdl/pkg"
"github.com/unstoppablemango/tdl/pkg/target"
uxv1alpha1 "github.com/unstoppablemango/tdl/pkg/unmango/dev/ux/v1alpha1"
Expand All @@ -23,12 +24,15 @@ func ParseArgs(args []string) (*uxv1alpha1.RunConfig, error) {
return nil, errors.New("no input specified")
}

log := log.With("args", args)
input := &uxv1alpha1.Input{}
if len(args) == 1 || args[inputIndex] == "-" {
log.Debug("choosing stdin")
input.Value = &uxv1alpha1.Input_Stdin{
Stdin: true,
}
} else {
log.Debugf("choosing input file: %s", args[inputIndex])
input.Value = &uxv1alpha1.Input_File{
File: &uxv1alpha1.FileInput{
Path: args[inputIndex],
Expand All @@ -42,10 +46,12 @@ func ParseArgs(args []string) (*uxv1alpha1.RunConfig, error) {
}

if len(args) > 2 {
log.Debugf("choosing output file: %s", args[outputIndex])
config.Output = &uxv1alpha1.RunConfig_Path{
Path: args[outputIndex],
}
} else {
log.Debug("choosing stdout")
config.Output = &uxv1alpha1.RunConfig_Stdout{
Stdout: true,
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/config/run/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ type reader struct {
media tdl.MediaType
}

// String implements tdl.Input.
func (r *reader) String() string {
return fmt.Sprintf("reader: %s", r.media)
}

func (r *reader) MediaType() tdl.MediaType {
return r.media
}
Expand Down
17 changes: 15 additions & 2 deletions pkg/config/run/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"io"
"io/fs"

"github.com/charmbracelet/log"
"github.com/spf13/afero"
tdl "github.com/unstoppablemango/tdl/pkg"
)
Expand All @@ -12,8 +13,14 @@ type writerOutput struct {
writer io.Writer
}

// String implements tdl.Output.
func (w *writerOutput) String() string {
return "writer"
}

func (w *writerOutput) Write(output afero.Fs) error {
return afero.Walk(output, "",
count := 0
err := afero.Walk(output, "",
func(path string, info fs.FileInfo, err error) error {
if err != nil {
return err
Expand All @@ -22,15 +29,21 @@ func (w *writerOutput) Write(output afero.Fs) error {
return nil
}

count++
file, err := output.Open(path)
if err != nil {
return err
}

_, err = io.Copy(w.writer, file)
n, err := io.Copy(w.writer, file)
log.Debugf("wrote %d bytes", n)

return err
},
)

log.Debugf("copied %d files", count)
return err
}

func WriterOutput(writer io.Writer) tdl.Output {
Expand Down
12 changes: 6 additions & 6 deletions pkg/conform/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@ import (
func AssertStdout(test *e2e.Test, output afero.Fs) {
By("opening test output")
expected, err := aferox.OpenSingle(test.Expected, "")
Expect(err).NotTo(HaveOccurred())
ExpectWithOffset(1, err).NotTo(HaveOccurred())

By("reading test output")
data, err := io.ReadAll(expected)
Expect(err).NotTo(HaveOccurred())
ExpectWithOffset(1, err).NotTo(HaveOccurred())

Expect(output).To(ContainFileWithBytes("stdout", data))
ExpectWithOffset(1, output).To(ContainFileWithBytes("stdout", data))
}

func AssertFile(path string) e2e.Assertion {
return func(test *e2e.Test, output afero.Fs) {
By("opening test output")
expected, err := test.Expected.Open(path)
Expect(err).NotTo(HaveOccurred())
ExpectWithOffset(1, err).NotTo(HaveOccurred())

By("reading test output")
data, err := io.ReadAll(expected)
Expect(err).NotTo(HaveOccurred())
ExpectWithOffset(1, err).NotTo(HaveOccurred())

Expect(output).To(ContainFileWithBytes(path, data))
ExpectWithOffset(1, output).To(ContainFileWithBytes(path, data))
}
}
10 changes: 5 additions & 5 deletions pkg/conform/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"

"github.com/charmbracelet/log"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

Expand All @@ -13,18 +12,19 @@ import (
)

func ItShouldPass(generator tdl.Generator, test *e2e.Test, assertions ...e2e.Assertion) {
if len(assertions) == 0 {
panic("test contained no assertions")
}

It(fmt.Sprintf("should pass: %s", test.Name), func(ctx context.Context) {
By("executing the generator")
output, err := generator.Execute(ctx, test.Spec)

Expect(err).NotTo(HaveOccurred())
ExpectWithOffset(1, err).NotTo(HaveOccurred())
By("performing the given assertions")
for _, assert := range assertions {
assert(test, output)
}
if len(assertions) == 0 {
log.New(GinkgoWriter).Warnf("no assertions for: %s", test.Name)
}
})
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ func (m MediaType) String() string {
}

type Input interface {
fmt.Stringer
io.Reader
MediaType() MediaType
}

type Output interface {
fmt.Stringer
Write(afero.Fs) error
}

Expand Down
Loading

0 comments on commit f566251

Please sign in to comment.