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

cmd/vet: eliminate use of Perl in tests #20032

Closed
josharian opened this issue Apr 18, 2017 · 23 comments

Comments

Projects
None yet
@josharian
Copy link
Contributor

commented Apr 18, 2017

See #20007 and discussion at CL 40950.

@josharian josharian added this to the Unplanned milestone Apr 18, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 18, 2017

Note that test/run.go has a Go port of errchk already.

func (t *test) errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {}
func (t *test) wantedErrors(file, short string) (errs []wantedError) {}
@niconegoto

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2017

@bradfitz I will work on this.

@SCKelemen

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2017

@niconegoto Are you still working on this?

@niconegoto

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2017

@SCKelemen Sorry.I don't have time to work on this now.

@grepory

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

If we moved a Go rewrite of errchk into its own package under test/errorcheck/ and made it buildable, we could both continue to use the errorcheck package directly from test/run.go and compile test/errorcheck/cmd into test/errchk for use in vet's tests. Does this seem reasonable?

I think we cannot move this under src/ somewhere, because test/run.go is run with the bootstrap go, right? So it's using not this stdlib? I'm still fuzzy on the build process.

@bradfitz or @niconegoto

@grepory

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

I'd prefer a solution that's just calling methods/funcs in both places, but I'm not sure if that's actually possible.

EDIT:

I should also say that this would feel weird to me in stdlib.

@ALTree

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

@grepory Putting errchk in the stdlib just to ease the testing process of vet is definitely a no-no IMHO. If everything else fails and/or we don't find a simple way to avoid duplication, I'd say last resort would be to just copy the two go functions we already ported in the vet_test file and use them from there.

@grepory

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2017

errcheck is used in more than vet test, iirc. I couldn't figure out a way to do it that didn't involve duplication, so I just decided to drop it for now. And yeah, I agree with you about errcheck in stdlib.

@ALTree

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

errcheck is used in more than vet test, iirc

A silly recursive grep in src/ for errchk (the name of the perl file) only shows results in ./cmd/vet/vet_test.go, but maybe I missed some uses.

@grepory

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2017

Probably not. I think I was conflating this with another issue I read which was "remove use of perl in build process" -- which is something to which we should all aspire.

@SCKelemen

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2017

I can work on this, if there is a decision on what to do.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2017

@SCKelemen Thanks. We should simply copy the errchk functionality, rewritten into Go, into cmd/vet. You can probably start with the functions @bradfitz mentioned in test/run.go. We should not create a new package for this. We will almost certainly want to adapt the Go code for the specific tool needs over time.

@SCKelemen

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2017

Okay, I'll work on this.

@forskning

This comment has been minimized.

Copy link

commented Nov 9, 2017

I wrote to Perl 5 Porters, here are the responses.

Shlomi Fish

Well, escaped left braces in regexes ("\{") should work fine in older versions of perl, so there is no good reason to avoid escaping them.

Jim Keenan

No matter what language you use, and no matter whether that's for ordinary code or for a project's test suite, that language is going to evolve over time. No language is 100% backwards-compatible for all time. That implies that periodically you may have to update your code to account for such issues.

Now, other things being equal, if the code in question is the core distribution of a computer language, you have more control if the core's test suite is written in that language itself. At the very least, you're likely to find out very quickly if changes in the language break the test suite. Indeed, when #20007 emerged earlier this year, I was surprised to learn that part of the golang test suite was written in Perl.

That being said, as Shlomi has already pointed out, the actual breakage in the golang test suite was small and easily fixed to work in both perl-5.26 and earlier versions.

@awoodbeck

This comment has been minimized.

Copy link
Contributor

commented Dec 28, 2017

@SCKelemen do you still have this issue?

@SCKelemen

This comment has been minimized.

Copy link
Contributor

commented Dec 28, 2017

@awoodbeck This has fallen off of my radar. My apologies. If you want to work on this, go for it. If not, I can try to work on this during the weekend.

@ysmolsky

This comment has been minimized.

Copy link
Member

commented May 22, 2018

FYI, I will try to handle it.

@forskning

This comment has been minimized.

Copy link

commented May 22, 2018

@ysmolsky

FYI, I will try to handle it.

Upgrading go/test/errchk for >= perl-5.26 or eliminating the use of Perl?

@ysmolsky

This comment has been minimized.

Copy link
Member

commented May 22, 2018

Eliminating the use of perl. Prototype was done, going to polish some edges and submit CL...

@gopherbot

This comment has been minimized.

Copy link

commented May 22, 2018

Change https://golang.org/cl/114176 mentions this issue: cmd/vet: eliminate use of Perl in tests

@forskning

This comment has been minimized.

Copy link

commented May 22, 2018

@ysmolsky

https://code.9front.org/hg/plan9front/file/8e57f29e99a3/sys/lib/kbmap/by

There is a golang port to Plan 9, and not per se golang related, but if the opportunity arises and someday you might check the by kbmap for accuracy, which I modified from a Plan 9 ru kbmap file, and submitted to 9front on 05 May 2016, though I never had tested the file, it would be appreciated.

https://code.9front.org/hg/plan9front/file/2241e3dd1a2c/sys/lib/kbmap/by

22 May 2018 change

@ysmolsky ysmolsky modified the milestones: Unplanned, Go1.11 May 23, 2018

@ysmolsky

This comment has been minimized.

Copy link
Member

commented May 24, 2018

Slightly related, should we try to eliminate errchk completely from Go tree?

I see that there are two tests use it:
https://github.com/golang/go/blob/master/test/fixedbugs/bug248.go
https://github.com/golang/go/blob/master/test/fixedbugs/bug345.go

Just by looking at them, I do not see easy way out since we cannot use errorCheck from test/run.go.

Any ideas?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 24, 2018

I think we can do it by adding a couple more actions to test/run.go. errchklinkdir and errchkcompile or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.