-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Activate previously skipped tests in Go target. #3842
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
base: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Terence Parr <[email protected]>
Signed-off-by: Terence Parr <[email protected]>
|
@jimidle I have reactivated some tests that are now failing. Can you take a look? Oh, I found another test that was ignored: AtnStatesSizeMoreThan65535. I don't know if that will succeed you will find out when you try to run the tests. |
|
Yes - I am looking now. Not sure if i can fix them all today, but let's see |
|
Hi @jimidle no problem. I'm going to try for a release this coming weekend because it's a long weekend here. We can leave this for after release if necessary. |
|
@parrt To fix these properly I will have to rejig the runtime structures. Which is something that I wish to do anyway, but cannot be done for this weekend's release. So, let's leave this issue open - you can assign it to me if you wish. After release |
|
OK let's leave it open and I will change the tag number. |
|
I will have the missing error token one done my tomorrow. I think that’s
quite important as it is out of step with the other targets.
See where I get to with the other two.
…On Tue, Aug 30, 2022 at 23:51 Terence Parr ***@***.***> wrote:
Hi @jimidle <https://github.com/jimidle> no problem. I'm going to try for
a release this coming weekend because it's a long weekend here. We can
leave this for after release if necessary.
—
Reply to this email directly, view it on GitHub
<#3842 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7TMHFV5W43EGLERQA4UDV3YUZDANCNFSM57Z4YOOQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I need to get a release inside google fairly soon; I was going to try this Friday with the existing release. Hmm... Maybe I figure out how to do that and then we can make another release with your fixes Jim? |
|
The three tests that fail here are because Go fell behind in the error
recovery strategy. It’s a fix that needs a bit of time to go through and
get correct. I haven’t had time to do that to date.
These tests though are purely about token insertion. Ironically, before and
after calling code that is commented “Jim Idle’s error recovery…. Blah”.
So they don’t affect the happy path. We can make another release without
them.
I will have time Monday and Tuesday next week if there any other go fixes
that you would like to get in.
This time we should not create the ‘v’ version of the tag. Only the tags
with the path in them. If this is a patch release, then we can leave in
place the code that isn’t in the v4 directory. If it is a bigger release,
then we can lose that code now, so that people don’t get confused.
…On Fri, Oct 14, 2022 at 00:58 Terence Parr ***@***.***> wrote:
I need to get a release inside google fairly soon; I was going to try this
Friday with the existing release. Hmm... Maybe I figure out how to do that
and then we can make another release with your fixes Jim?
—
Reply to this email directly, view it on GitHub
<#3842 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7TMC7NYHA6TKMM4VSDGTWDA5SDANCNFSM57Z4YOOQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I am doing the RUST target works in rust-target (antlr4rust#4). the test case 'AtnStatesSizeMoreThan65535' is failed. I find the same issue of Go target. PR 4883 |
Some were turned off due to performance I think.