Skip to content
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.

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

genny v3 proposal: change API to make usage easier #43

Open
fasmat opened this issue Dec 26, 2021 · 1 comment
Open

genny v3 proposal: change API to make usage easier #43

fasmat opened this issue Dec 26, 2021 · 1 comment
Assignees
Labels
proposal A suggestion for a change, feature, enhancement, etc
Milestone

Comments

@fasmat
Copy link
Member

fasmat commented Dec 26, 2021

genny v2 could be improved imho by making the API feel more like one is writing go code using the standard library instead of an abstraction of such. This issue serves as a collection of sorts for those API proposals:

remove all usages of packr

With the io/fs package introduced in go 1.16 we can remove all usages of packr and replace them with their new equivalents.

genny.Runner should not change behavior after instantiation

All fields of genny.Runner should be private to ensure they are only set during instantiation of the object and not modified afterwards. Provide a New function with options like WithLogger, WithExecFn, etc. to set those fields. WithDisk can be used by gentest.NewRunner to return a runner for testing together with a reference to the virtual disk for seeding.

type Runner struct {
        logger     LoggerInterface    // Instead of a concrete type Logger should just be an interface
	context    context.Context
	execFn     func(*exec.Cmd) error
	fileFn     func(File) (File, error)
	chdirFn    func(string, func() error) error
	deleteFn   func(string) error
	requestFn  func(*http.Request, *http.Client) (*http.Response, error)
	lookPathFn func(string) (string, error)
	root       string
	disk       *Disk
	steps      map[string]*Step
	moot       *sync.RWMutex
	results    Results
	curGen     *Generator
}

genny.Runner should abstract usage of virtual / real files

The disk object represents a virtual file system that is used in a dry run to simulate the actual file system. It should not be accessed from outside genny.Runner to prevent the user from thinking that modifying the disk actually changes something on the hard drive when a wet runner is used. Instead most of it's functionality will be added to genny.Runner:

// replaces File. Create creates a new file the returned file is a proxy of either os.File or a virtual file
// mkdir and mkdirall behave like their os package equivalents or create directories on the virtual disk
func (r *Runner) Create(name string) (*File, error)
func (r *Runner) Mkdir(name string, perm os.FileMode) error
func (r *Runner) MkdirAll(path string, perm os.FileMode) error

// new method signature, runner remembers where it chdir'd to correctly access to the virtual disk after changing the directory
// it also changes back to the original directory when the runner finishes
func (r *Runner) Chdir(dir string) error

// replaces from FindFile. tries to open a file on disk or the virtual file system
func (r *Runner) Open(name string) (*File, error)

// renamed from delete to be more in line with the "os" package
func (r *Runner) Remove(name string) error
func (r *Runner) RemoveAll(path string) error

// updated method signature to be more in line with "net/http" package:
func (r *Runner) DoRequest(req *http.Request) (*http.Response, error)
func (r *Runner) DoRequestWithClient(c *http.Client, req *http.Request) (*http.Response, error)

// unchanged
func (r *Runner) Exec(cmd *exec.Cmd) error
func (r *Runner) FindStep(name string) (*Step, error)
func (r *Runner) LookPath(file string) (string, error)
func (r *Runner) ReplaceStep(name string, s *Step) error
func (r *Runner) Results() Results
func (r *Runner) Run() error
func (r *Runner) Steps() []*Step
func (r *Runner) With(g *Generator) error
func (r *Runner) WithFn(fn func() (*Generator, error)) error
func (r *Runner) WithGroup(gg *Group)
func (r *Runner) WithNew(g *Generator, err error) error
func (r *Runner) WithRun(fn RunFn)
func (r *Runner) WithStep(name string, step *Step) error

gentest package updates

To make it easier to setup a virtual filesystem in tests now gentest.NewRunner() returns access to the internal disk of the runner that is used for testing:

func NewRunner() (*genny.Runner, *genny.Disk)

implementation of io/fs interfaces

Disk should implement the following interfaces; Runner should probably implement most of these interfaces too:

fs.FS{}         // because it is a FS
fs.StatFS{}     // allows to walk the Disk with fs.WalkDir
fs.GlobFS{}     // to easily find all files matching a pattern
fs.ReadDirFS{}  // equivalent of os.ReadDir; allows more efficient walking
fs.ReadFileFS{} // equivalent of os.ReadFile
fs.SubFS{}      // allows the creation of sub-filesystems

Returned virtual files and directories should implement the io/fs interfaces like their os equivalents:

fs.File{}
fs.DirEntry{}
@fasmat fasmat self-assigned this Dec 26, 2021
@sio4 sio4 added this to the v3 milestone Sep 5, 2022
@sio4 sio4 added the proposal A suggestion for a change, feature, enhancement, etc label Sep 5, 2022
@tbruyelle
Copy link

tbruyelle commented Jan 5, 2023

Hey @fasmat,

I just figured out that current runners doesn't support fs.FS, so this issue would be a great enhancement.

What I have in mind is a single NewRunner(fs.FS) :

  • if you want a dry runner use NewRunner(fstest.MapFS{})
  • if you want a wet runner use NewRunner(os.DirFS("."))

That said DryRunner and WetRunner could be kept, but they will just call NewRunner with the appropriate fs.

What do you think ?

EDIT: seems like I went a little too fast, fs.FS is only a readable interface, so it cannot be used by a runner. Maybe this will change in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A suggestion for a change, feature, enhancement, etc
Projects
None yet
Development

No branches or pull requests

3 participants