[Go] Fix for issue 3557 -- get "go test" working again

[kaby76]

What does this PR fix?

• [Go] "go test" does not pass #3557 (Go target)

Discussion

This is a fix for the Go runtime so that "go test" works. The problem is that some of the runtime tests are within the runtime itself, and I don't think they are tested in GitHub Actions.

The change is essentially a couple of test files in the runtime. One of the files didn't explicitly state the "LexerA.g4" grammar, so I inserted a comment so people don't need to guess WTF it is.

Why is this PR important?

It's blocking testing for another PR I want to make.

[kaby76]

Prereq for #3548

[KvanTTT]

Where did you take it from? Probably it should be extracted to a separate file.

[kaby76]

Of course it should be in a file. But, so should the grammar in the comment here.

But, even if we decide to leave it here, and pull out the grammars into files, and generate the lexers for testing, the generated code will not build. That's because we have the lovely global variable problem. And, rather than deal with the problem in an intelligent manner, the output from the tool is "cleverly" hand-edited.

Even if we fix #3452, API tests should not even go here. How can one test the visibility of the API from within the package? It's all just wrong. The tests should not be here. It needs to go into the runtime-testsuite directory.

Oh yes, the grammar I got was just an educated guess after looking at the output that was there before. But, I didn't actually analyze the automaton produced representing in the tables. So, who knows if this is right. The "experts" are so much smarter than I. I only have a ~1/2 century of programming that I have done, but it's nothing.

[KvanTTT]

So, who knows if this is right. The "experts" are so much smarter than I. I only have a ~1/2 century of programming that I have done, but it's nothing.

Everyone can mistake despite the big experience as well as suggest a good solution. And it's okay.

[kaby76]

It's normal to make mistakes. We are all human. It's wrong to not take ownership. This has been here for a long time, enough time to reflect and try to fix.

I think it best to add the code as is. #3452 / #3264 needs to be addressed post 4.10. I have a fix for this, but it needs to be integrated with a fix with #3446, which I also have.

[parrt]

thanks!

[parrt]

@kaby76 let me know if you need anything else :)

[jimidle]

Guys - I don't think we should go down this path for Go testing. I think it needs me to dedicate some serious time to write Go runtime tests in Go idiomatic form. I am going to do some serious rummaging around in the go runtime in the next few weeks, and when I have got the performance where I think it should be, then I will look at stuff like this.

Talking of performance, when I get some changes in to the dev branch, perhaps you will have time to run the comparison benchmark you have @kaby76 ? That seems some serious effort you put in there. I would like to get Go performance to be top of the tree. I know what is wrong, but some of the go stuff will require quite a bit of, shall we say, "correcting" to get rid of useless memory allocations and the like.

go test./... does work from the /v4 code, but there are valuable tests in there right now.

I suggest that we close this. BTW - I am just going through all PRs and issue that pertain to Go at the moment so I can clean them up/rework them/ask for them to be closed. Start with a clean slate so to speak.

[kaby76]

@jimidle I'll be looking at performance for Go soon. My plan is to: clean up and extend the test driver generator used in testing grammars-v4; add "individual" parsing back into selected targets and grammars, because "grouped" parsing (aka "warm-up") doesn't help for targets like PHP but the grammar "works" otherwise); add in parse tree comparison for all tests across all targets; finally, set up tests for performance across targets for selected grammars. It'll likely be a few weeks before performance tests are written.

[KvanTTT]

I think it needs me to dedicate some serious time to write Go runtime tests in Go idiomatic form.

I'm not sure it's correct. We have several runtimes and ideally tests should be compatible with all of them. It's not idiomatic form but universal.

[parrt]

We definitely want to keep the general tests but you're free to make whatever extra tests you want in your runtime directories.

[jimidle]

I should have explained better. The kind of tests that would be here are low level things specifically to go. You are 100% correct that we should not try to recreate the higher level test bed here. Nor would I wish to. These are low level unit and regression tests, and perhaps more importantly, performance/memory tests that keep history and point out any swings for the worst after changes. The other thing that goes here is Example tests, which are auto included in the Go documentation.

…

On Sat, Mar 4, 2023 at 23:20 Ivan Kochurkin ***@***.***> wrote: I think it needs me to dedicate some serious time to write Go runtime tests in Go idiomatic form. I'm not sure it's correct. We have several runtimes and ideally tests should be compatible with all of them. It's not idiomatic form but universal. — Reply to this email directly, view it on GitHub <#3558 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7TMFCIU6I243P4TPQ763W2NMSDANCNFSM5PFIKXXQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>