Feature ASH language server features

I did ant daily (to update and build 20838) and then ant test. The latter failed.

Code: 
    [junit] Testcase: testScriptValidity[Script with bom](net.sourceforge.kolmafia.textui.ParserTest):  Caused an ERROR
    [junit] Script parsing error ()
    [junit] net.sourceforge.kolmafia.textui.ScriptException: Script parsing error ()
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parseException(Parser.java:4698)
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parse(Parser.java:224)
    [junit]     at net.sourceforge.kolmafia.textui.ParserTest.testScriptValidity(Unknown Source)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

I'm hoping it is a trivial result of checking in and the running tests and that it is related to this work.
 
Hard to test, Clover doesn't even let me run them...
Code: 
Buildfile: C:\Users\Frederic\Desktop\KoLmafia-workspace\kolmafia\build.xml

init:

svn-version:

gitsvn-version:

version:
     [echo] Current revision: 20838M

compile-deps:

compile:

svn-version:

gitsvn-version:

version:
     [echo] Current revision: 20838M

set.version:
    [javac] Compiling 1 source file to C:\Users\Frederic\Desktop\KoLmafia-workspace\kolmafia\build
    [javac] Ignoring source, target and bootclasspath as release has been set

svn-version:

gitsvn-version:

version:
     [echo] Current revision: 20838M

unset.properties:

test-compile:
    [javac] Compiling 1 source file to C:\Users\Frederic\Desktop\KoLmafia-workspace\kolmafia\build
    [javac] warning: [options] bootstrap class path not set in conjunction with -source 8
    [javac] 1 warning

test:
    [junit] Clover estimates having saved around 6 hours on this optimized test run. The full test run takes approx. 6 hours
    [junit] Clover included 5 test classes in this run (total # test classes : 17)
    [junit] Running net.sourceforge.kolmafia.request.ChatRequestTest
    [junit] Testsuite: net.sourceforge.kolmafia.request.ChatRequestTest
    [junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2,482 sec
    [junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2,482 sec
    [junit]
    [junit] Running net.sourceforge.kolmafia.utilities.IntWrapperTest
    [junit] Testsuite: net.sourceforge.kolmafia.utilities.IntWrapperTest
    [junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0,052 sec
    [junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0,052 sec
    [junit]
    [junit] Running net.sourceforge.kolmafia.utilities.KoLDatabaseTest
    [junit] Testsuite: net.sourceforge.kolmafia.utilities.KoLDatabaseTest
    [junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0,031 sec
    [junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0,031 sec
    [junit]
    [junit] Running net.sourceforge.kolmafia.utilities.LowerCaseEntryTest
    [junit] Testsuite: net.sourceforge.kolmafia.utilities.LowerCaseEntryTest
    [junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2,229 sec
    [junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2,229 sec
    [junit]
    [junit] Running net.sourceforge.kolmafia.utilities.RollingLinkedListTest
    [junit] Testsuite: net.sourceforge.kolmafia.utilities.RollingLinkedListTest
    [junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0,104 sec
    [junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0,104 sec
    [junit]
    [junit] Testcase: itShouldBehaveForABulkAdd(net.sourceforge.kolmafia.utilities.RollingLinkedListTest):SKIPPED: Class under test needs to be augmented

BUILD SUCCESSFUL
Total time: 14 seconds
Press any key to continue . . .

Terminal will be reused by tasks, press any key to close it.

@fronobulax Could you apply this patch and restart the tests? It's the error message you got, modified to give more information (one of the things I planned to add later, anyway).
It should help us find what's happening.
 

Attachments

So, you may have noticed that in one of the changes (I think the big one, 20835) I made a change to the parameterized test name. OpenClover doesn't want to read it without extra effort, but it was just providing an index previously...

Anyways, it looks like the test case that's failing for frono is "Script with bom", which looks like
Code: 
\ufeff    'hello world'

Although, something I'm observing:

Code: 
            if ( line.length() > 0 &&
                 line.charAt( 0 ) == Parser.BOM )
Should this be line.codePointAt, seeing as the BOM is a unicode codepoint that's actually 2 bytes?

@fronobulax , can you see if the test passes with that change?

edit: we'd presumably also need to change BOM to be an int or other type that can actually hold 2 bytes.
 
Code: 
    [junit] Testcase: testScriptValidity[Script with bom](net.sourceforge.kolmafia.textui.ParserTest):  Caused an ERROR
    [junit] Script parsing error: we didn't reach the end of the file.
    [junit] Current line reads ?    'hello world'
    [junit] Next line is end of file. ()
    [junit] net.sourceforge.kolmafia.textui.ScriptException: Script parsing error: we didn't reach the end of the file.
    [junit] Current line reads ?    'hello world'
    [junit] Next line is end of file. ()
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parseException(Parser.java:4703)
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parse(Parser.java:224)
    [junit]     at net.sourceforge.kolmafia.textui.ParserTest.testScriptValidity(Unknown Source)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

Patch applied and run.

At least 12 hours before I will get back to this but will try and help.
 
While this gets sorted out, here are a variety of small patches:
  1. Parser's public static fields "probably" ought to be final
  2. Removal of redundant method parsePostCall.
    • Said method was called at the very end of parseCall and parseInvoke.
    • Both parseCall and parseInvoke are (*only*) called by parseValue.
    • parseValue ends with [[[paused to answer to frono's post]]]
      *ahem*...
    • parseValue ends with the same logic as parsePostCall, in addition to making it act on an additional case.
    • This effectively makes parsePostCall redundant, as anything it does would have been done anyway in parseValue
Code_YliUYNpwKu.png
 

Attachments

Last edited:
fronobulax said:
Code: 
    [junit] Testcase: testScriptValidity[Script with bom](net.sourceforge.kolmafia.textui.ParserTest):  Caused an ERROR
    [junit] Script parsing error: we didn't reach the end of the file.
    [junit] Current line reads ?    'hello world'
    [junit] Next line is end of file. ()
    [junit] net.sourceforge.kolmafia.textui.ScriptException: Script parsing error: we didn't reach the end of the file.
    [junit] Current line reads ?    'hello world'
    [junit] Next line is end of file. ()
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parseException(Parser.java:4703)
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parse(Parser.java:224)
    [junit]     at net.sourceforge.kolmafia.textui.ParserTest.testScriptValidity(Unknown Source)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
Click to expand...
So, what I think I'm seeing here, is that the Byte Order Mark was somehow not skipped...
 
More accurately, it seems like the Byte Order Mark was not *recognized*...
Which leads to it not being trimmed, which leads to none of the parsing methods finding anything.
parseScope then assumed that "finding nothing means we're at the end of the file".
 
This seems non-portable what with endianness unless we do something like
Code: 
    private static final String BOM = "\ufeff";
...
            if ( line.length() > 0 &&
                 line.startsWith( Parser.BOM ) )
            {
                line = line.substring( Parser.BOM.length() );
                offset += Parser.BOM.length();
            }

There's a secondary issue, namely that substring(1) is the wrong thing to do here, when our BOM is actually 2 characters wide.
 
heeheehee said:
So, you may have noticed that in one of the changes (I think the big one, 20835) I made a change to the parameterized test name. OpenClover doesn't want to read it without extra effort, but it was just providing an index previously...

Anyways, it looks like the test case that's failing for frono is "Script with bom", which looks like
Code: 
\ufeff    'hello world'

Although, something I'm observing:

Code: 
            if ( line.length() > 0 &&
                 line.charAt( 0 ) == Parser.BOM )
Should this be line.codePointAt, seeing as the BOM is a unicode codepoint that's actually 2 bytes?

@fronobulax , can you see if the test passes with that change?

edit: we'd presumably also need to change BOM to be an int or other type that can actually hold 2 bytes.
Click to expand...


I made the codePointAt change in Parser and the error didn't change. Sorry. I am going to revert the patch and my change and see what happens tomorrow.
 
Code_WrVHS5DBj4.pngIs this platform-dependant or something? Because I'm not getting this (also how would it have passed the test for us if that was the case?)
 
I... I don't know. It shouldn't be? Maybe a Java version / distribution? or something OS-dependent?

I spent some time staring at the Java docs re: char / String / substring / length. The BOM is part of the "Basic Multilingual Plane" (so, fits in a single char), and all of these methods are defined in terms of chars (which are evidently 2 bytes in Java... too much C/C++ for me...). I'm out of ideas.
 
An observation: in the build.xml, the javac task in compile / compile-deps sets encoding="utf-8" but not in test-compile or coverage-compile.

I've committed r20839 in the hopes that it makes a difference. I'm skeptical, though.
 
Reverted local changes and updated to 20839. Same test failure.

I (re)applied the test patch with more informative logging.

Code: 
    [junit] Testcase: testScriptValidity[Script with bom](net.sourceforge.kolmafia.textui.ParserTest):  Caused an ERROR
    [junit] Script parsing error: we didn't reach the end of the file.
    [junit] Current line reads ?    'hello world'
    [junit] Next line is end of file. ()
    [junit] net.sourceforge.kolmafia.textui.ScriptException: Script parsing error: we didn't reach the end of the file.
    [junit] Current line reads ?    'hello world'
    [junit] Next line is end of file. ()
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parseException(Parser.java:4703)
    [junit]     at net.sourceforge.kolmafia.textui.Parser.parse(Parser.java:224)
    [junit]     at net.sourceforge.kolmafia.textui.ParserTest.testScriptValidity(Unknown Source)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

Since the question was raised, I'm on Windows 10 and
Code: 
$ java -version
java version "13.0.2" 2020-01-14
Java(TM) SE Runtime Environment (build 13.0.2+8)
Java HotSpot(TM) 64-Bit Server VM (build 13.0.2+8, mixed mode, sharing)
 
I've tried:

java-16-openjdk
java-16-jdk (Oracle Java)
java-11-openjdk
java-8-openjdk

all on Linux, without any test failures. My guess is that it's something about Windows, but I'll need to spend some time later to figure that out.
 
Well I am using Oracle 13 :cool:

I recall tracking down and fixing a bug in our product that ultimately was due to what Windows considered the default character set and what other character sets were available. That was long ago but maybe if I actually try and look at the code under test my memory will be jogged.
 
Well...

I got PO'd at my IDE (because I couldn't configure it to run KoLmafia in the debugger. My fault because of how I migrated to a new machine). I got that fixed and ran ParserTest in the debugger. No test error! But ant test at both a Cygwin command line (my usual) and a Windows command prompt both failed. My eyes are glazing over so break time.
 
Hypothesis - the IDE swallows exceptions before they get to the test code. Therefore the test will pass in the IDE but fail at the command line.

Circa line 222 in Parser the debugger thinks the currentToken is ";". This confuses me since the string being passed to the parser does not contain the ; character and so something programmatically may have added it as a token.

At the same time and place the currentLine is reported as "null", specifically the string "null" and not the expected null reference. That could be correct but it seems counter intuitive. I believe String.valueOf(null) returns "null" so perhaps something that needs to check for null is not doing so;

Break time.
 
fronobulax said:
Hypothesis - the IDE swallows exceptions before they get to the test code. Therefore the test will pass in the IDE but fail at the command line.

Circa line 222 in Parser the debugger thinks the currentToken is ";". This confuses me since the string being passed to the parser does not contain the ; character and so something programmatically may have added it as a token.

At the same time and place the currentLine is reported as "null", specifically the string "null" and not the expected null reference. That could be correct but it seems counter intuitive. I believe String.valueOf(null) returns "null" so perhaps something that needs to check for null is not doing so;

Break time.
Click to expand...
The "end of file" Line (or what we marked as such) contains a single Token, with a value of ";"
You can see it being assigned on line 4571
 
fredg1 said:
The "end of file" Line (or what we marked as such) contains a single Token, with a value of ";"
You can see it being assigned on line 4571
Click to expand...

OK. In my days of parsing we didn't add anything to what was being parsed and so I wasn't sure it was deliberate.

I'm still poking at curretLine being "null" in the debugger and still wish I understood while a test that throws an (unexpected) Exception doesn't fail in the IDE.
 
You must log in or register to reply here.
Back
Top