Test fixes #9

willfaught · 2016-05-27T06:46:17Z

Fix various tests
Minor (accidental) clean up
Change import paths from antlr/antlr4 to pboyer/antlr4 to ease testing

Also minor formatting.

Also: - Change "antlr4" imports to "github.com/antlr/antlr4/runtime/Go/antlr" - Remove parser prefix from Property and pred calls - Change TParser.X tokens to TParserX

pboyer · 2016-05-27T16:33:24Z

runtime-testsuite/test/org/antlr/v4/test/runtime/go/BaseTest.java

@@ -391,7 +391,7 @@ public String execModule(String fileName) {
 			ProcessBuilder builder = new ProcessBuilder(goPath, "run", modulePath,
 					inputPath);
 			builder.environment().put("GOPATH",
-					runtimePath + File.pathSeparator + tmpdir);
+					builder.environment().get("GOPATH") + ":" + runtimePath + File.pathSeparator + tmpdir);


What happens if this environment variable is not defined? Is it possible we could throw a more verbose message or try something else?

Good question. I'm not familiar with the API, I actually guessed there's a get method based on the use of the put method. Open to suggestions.

And also things stopped breaking after the change. :)

It also just occurred to me that : isn't cross-platform.

$GOPATH ends up as :<runtimePath>:<tmpdir> (I guess), i.e. the first element is the empty string. Go freaks out with an error message along the lines of "GOPATH needs to be absolute, but this is relative" (or something).

Edit: exec stderrVacuum: go: GOPATH entry is relative; must be absolute path: "null".

I think an empty path element defaults to the current directory. Working on a fix to avoid that.

I think I was wrong with empty path, instead it becomes literal string null.

pboyer · 2016-05-27T16:35:38Z

Thanks @willfaught! Just one comment and lgtm.

willfaught · 2016-05-27T18:16:50Z

This still seems to not work for some reason, just wanted to put up what I was thinking. This produces a GOPATH of /Users/Will/Developer/go/Users/Will/Developer/go/src/github.com/willfaught/antlr4/runtime-testsuite/target/classes/Go/src/github.com/willfaught/antlr4/runtime/Go/antlr, which isn't right. The path separator doesn't seem to show up.

(Updated)

jwkohnen · 2016-05-27T18:28:22Z

It doesn't work because of our go import path. Peter's old system used runtimePath which would yield $CWD/runtime-testsuite/target/classes/Go and below that it had the subdir src/antlr4 with the runtime sources, hence go could find the import path antlr4 there.

Now we use a complete import path, such that we have to put the whole repository under our $GOPATH/src/github.com/pboyer. That makes runtime obsolete, but now we messed up automated builds (with $GOPATH unset), because only go developers use go-specific project directory layouts.

As of now I am happy with that, but we need a better idea. :)

willfaught · 2016-05-27T18:29:01Z

Whoops, misread the Javadoc. Using File.pathSeparator worked.

willfaught · 2016-05-27T18:32:12Z

That makes runtime obsolete, but now we messed up automated builds (with $GOPATH unset), because only go developers use go-specific project directory layouts.

Can you give more detail? When/where are these builds happening with GOPATH unset?

jwkohnen · 2016-05-27T18:35:47Z

One example, when a build w/o GOPATH happens is travis (see also #12), or whenever a non-go-developer tries this at home.

This is an example error:

dir /tmp/TestCompositeParsers-1464374124472/parser
exec stderrVacuum: parser/m_base_listener.go:4:8: cannot find package "github.com/pboyer/antlr4/runtime/Go/antlr" in any of:
    /opt/go/src/github.com/pboyer/antlr4/runtime/Go/antlr (from $GOROOT)
    /home/wjk/go/antlr/src/github.com/pboyer/antlr4/runtime-testsuite/target/classes/Go/src/github.com/pboyer/antlr4/runtime/Go/antlr (from $GOPATH)
    /tmp/TestCompositeParsers-1464374124472/src/github.com/pboyer/antlr4/runtime/Go/antlr

willfaught · 2016-05-27T18:37:53Z

Not sure what exec stderrVacuum is doing, but can we change it to set GOPATH?

jwkohnen · 2016-05-27T18:44:32Z

The tests use readers (stdoutVacuum and stderrVacuum that read STDERR and STDOUT in order to assert/compare each output with expected outputs. Because go complains, there is unexpected stuff in stderrVacuum.

The spot where you did the null fix is exactly where we can set the GOPATH as desired. The thing is, when the "external" GOPATH is empty and the user is a go developer, then s/he should set GOPATH correctly. When the user is any non-go developer or the travis build host, there is no meaningful value that we can set. At least, I have no idea what to use. Yet.

My take is, this is a problem that needs to be addressed, but it is not related to this PR. It just became apparent at this point.

willfaught · 2016-05-27T18:44:37Z

Ah, I think I see the issue. Basically, for all users, we can't ensure the repo appears in a structure like .../src/github.com/antlr/antlr4 such that we can just set GOPATH=...

I think it's reasonable that if you want to compile/test the Go target, you need Go installed and configured correctly. I wouldn't expect the C# target to work if I didn't have it installed.

jwkohnen · 2016-05-27T18:48:16Z

Yes, that's right.

Having go installed is not enough. It's also mandatory that the the git checkout is already inside a go-eligible directory. I think that is a hurdle that we should remove somehow. At least I have no idea how this could work with travis otherwise.

willfaught · 2016-05-27T18:55:56Z

In the case of Travis, we can set GOPATH to a temporary directory then go get the antlr4 repo. Or symlink the existing runtime lib into it.

On May 27, 2016, at 11:48 AM, Wolfgang Johannes Kohnen notifications@github.com wrote:

Yes, that's right.

Having go installed is not enough. It also mandatory that the the git checkout is already inside a go-eligible directory. I think that is a hurdle that we should remove somehow. At least I have no idea how this could work with travis otherwise.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

willfaught · 2016-05-27T19:18:32Z

@pboyer I think your point has been addressed. As @wjkohnen said, enabling Travis builds is a separate issue.

Update DefaultErrorStrategy.cpp

willfaught added 6 commits May 26, 2016 23:27

Rename Lexer.mode to setMode

10eb1a1

Also minor formatting.

Rename "this" receiver to "t"

92793a9

Change package alias antlr4 to antlr

3487776

Fix test GOPATH env var to include current GOPATH

3434e80

Also: - Change "antlr4" imports to "github.com/antlr/antlr4/runtime/Go/antlr" - Remove parser prefix from Property and pred calls - Change TParser.X tokens to TParserX

Change antlr/antlr4 to willfaught/antlr4

8d863ea

Change willfaught/antlr4 to pboyer/antlr4

9562d87

jwkohnen mentioned this pull request May 27, 2016

Timeout tests and GOROOT again #10

Closed

pboyer reviewed May 27, 2016
View reviewed changes

Handle empty current GOPATH when setting GOPATH

7e8fb7a

Change File.separator to File.pathSeparator

a0314a8

jwkohnen mentioned this pull request May 27, 2016

TODO: enable travis builds and seamless non-go builds #13

Closed

pboyer merged commit 6ddc7c9 into pboyer:master May 27, 2016

millergarym pushed a commit to millergarym/antlr4 that referenced this pull request Nov 16, 2016

Merge pull request pboyer#9 from nuuman/patch-1

a6699ea

Update DefaultErrorStrategy.cpp

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test fixes #9

Test fixes #9

willfaught commented May 27, 2016

pboyer May 27, 2016

willfaught May 27, 2016 •

edited

Loading

willfaught May 27, 2016

willfaught May 27, 2016

jwkohnen May 27, 2016 •

edited

Loading

willfaught May 27, 2016

jwkohnen May 27, 2016

pboyer commented May 27, 2016

willfaught commented May 27, 2016 •

edited

Loading

jwkohnen commented May 27, 2016 •

edited

Loading

willfaught commented May 27, 2016

willfaught commented May 27, 2016

jwkohnen commented May 27, 2016

willfaught commented May 27, 2016

jwkohnen commented May 27, 2016

willfaught commented May 27, 2016 •

edited

Loading

jwkohnen commented May 27, 2016 •

edited

Loading

willfaught commented May 27, 2016

willfaught commented May 27, 2016

Test fixes #9

Test fixes #9

Conversation

willfaught commented May 27, 2016

pboyer May 27, 2016

Choose a reason for hiding this comment

willfaught May 27, 2016 • edited Loading

Choose a reason for hiding this comment

willfaught May 27, 2016

Choose a reason for hiding this comment

willfaught May 27, 2016

Choose a reason for hiding this comment

jwkohnen May 27, 2016 • edited Loading

Choose a reason for hiding this comment

willfaught May 27, 2016

Choose a reason for hiding this comment

jwkohnen May 27, 2016

Choose a reason for hiding this comment

pboyer commented May 27, 2016

willfaught commented May 27, 2016 • edited Loading

jwkohnen commented May 27, 2016 • edited Loading

willfaught commented May 27, 2016

willfaught commented May 27, 2016

jwkohnen commented May 27, 2016

willfaught commented May 27, 2016

jwkohnen commented May 27, 2016

willfaught commented May 27, 2016 • edited Loading

jwkohnen commented May 27, 2016 • edited Loading

willfaught commented May 27, 2016

willfaught commented May 27, 2016

willfaught May 27, 2016 •

edited

Loading

jwkohnen May 27, 2016 •

edited

Loading

willfaught commented May 27, 2016 •

edited

Loading

jwkohnen commented May 27, 2016 •

edited

Loading

willfaught commented May 27, 2016 •

edited

Loading

jwkohnen commented May 27, 2016 •

edited

Loading