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

feat(gexec) Add CompileTest functions. Close #410 #411

Merged
merged 3 commits into from Feb 25, 2021
Merged

feat(gexec) Add CompileTest functions. Close #410 #411

merged 3 commits into from Feb 25, 2021

Conversation

holyhope
Copy link
Contributor

@holyhope holyhope commented Feb 6, 2021

Adds followinf methods:

  • CompileTest(packagePath string, args ...string) (compiledPath string, err error)
  • CompileTestWithEnvironment(packagePath string, env []string, args ...string) (compiledPath string, err error)
  • CompileTestIn(gopath string, packagePath string, args ...string) (compiledPath string, err error)

Signed-off-by: Pierre Péronnet pierre.peronnet@ovhcloud.com

@holyhope
Copy link
Contributor Author

holyhope commented Feb 6, 2021

I changed name of functions of #410. I suggested BuildTest..., but implemented CompileTest... so it matches the -c flags of go test and there is no confusion between a TestBuild(*testing.T) method and the one I implemented here.

@holyhope
Copy link
Contributor Author

holyhope commented Feb 6, 2021

Thanks to the last commit, gexec is now able to get test package before compile it :-)

Adds following methods:
- CompileTest(packagePath string, args ...string) (compiledPath string, err error)
- CompileTestWithEnvironment(packagePath string, env []string, args ...string) (compiledPath string, err error)
- CompileTestIn(gopath string, packagePath string, args ...string) (compiledPath string, err error)

Signed-off-by: Pierre Péronnet <pierre.peronnet@ovhcloud.com>
This solve the issue in test where suite scoped firefly binary
was removed by CleanupBuildArtifacts() in build_test.go

Signed-off-by: Pierre Péronnet <pierre.peronnet@ovhcloud.com>
Adds following methods:
- GetAndCompileTest(packagePath string, args ...string) (compiledPath string, err error)
- GetAndCompileTestWithEnvironment(packagePath string, env []string, args ...string) (compiledPath string, err error)
- GetAndCompileTestIn(gopath string, packagePath string, args ...string) (compiledPath string, err error)

Signed-off-by: Pierre Péronnet <pierre.peronnet@ovhcloud.com>
@holyhope
Copy link
Contributor Author

ping @onsi

@onsi
Copy link
Owner

onsi commented Feb 11, 2021

whoops sorry! thanks for the ping (and the code!) - will take a look today.

@blgm
Copy link
Collaborator

blgm commented Feb 11, 2021

I've had a very cursory look at this and did have two thoughts (which are very much thoughts rather than objections):

  • with regard to GetAndCompileTest(), is there an argument that Get() could be independent, so it could be chained with CompileTest() or with something else?
  • $GOPATH is on the way to being deprecated. I can see that this PR builds consistency with previous Gexec behaviour, and I do like being consistent. But I was wondering whether we might end up deprecating some of these functions quite soon when (or if) $GOPATH is removed?

@holyhope
Copy link
Contributor Author

Hello @blgm,

Thank you for taking care of my PR 👍

  • Get() Could be independent but atm it will be used only for CompileTest().
    An valid argument would be to implement more go commands such as go vet?
    Not sure about the terms chained with, I did not imagine to have chained method Get().CompileTest(), I rather expected

    func Test() {
      Get(packageName)
      CompileTest(packageName)
    }

    I agree that the first one looks nice. :-)
    I am not sure that a new structure should be introduced to handle the result of Get().
    WDYT, should I introduce this structure? Suggestion:

    type GoGet interface{
      CompileTest(args ...string) (string, error)
      CompileTestWithEnv(envs []string, args ...string) (string, error)
      Build(args ...string) (string, error)
      BuildWithEnv(envs []string, args ...string) (string, error)
    }
    
    func Get(packagePath) GoGet

    BTW in order to nicely chain methods, the GoGet objct will need to store the error in order to be raised with a final functions.

  • About $GOPATH I have no idea, I am sorry to tell you that I did not event know that it is deprecated. ¯\_(ツ)_/¯

@onsi
Copy link
Owner

onsi commented Feb 25, 2021

Thanks @holyhope - I expect the $GOPATH stuff will need some attention at some point. For now I'm happy to pull this in as is.

@onsi onsi merged commit 47c613f into onsi:master Feb 25, 2021
@holyhope holyhope deleted the 410/compile-test branch February 25, 2021 23:20
This was referenced Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants