[RFC PATCH] Catch throwable in sequence command and add more data

[taylor.smock]

I've spent way to much time trying to debug sequence commands.
In error reports, it would be nice if (at a minimum) the current sequence index is shown, and the actual sequence commands.

The attached patch shows the sequence name, the description of the current command, the current index, all of the command classes, and the command class descriptions.

[simon04]

In 16681/josm:

fix #19407 - SequenceCommand: catch throwable in sequence command and add more data (patch by taylor.smock, modified)

[simon04]

In 16682/josm:

see #19407 - Add SequenceCommandTest.testCreateReportedException

[Stereo]

This currently fails with all versions of Java on Windows, see ​https://github.com/grischard/josm/runs/808886525?check_suite_focus=true

[taylor.smock]

Replying to Stereo:

This currently fails with all versions of Java on Windows, see ​https://github.com/grischard/josm/runs/808886525?check_suite_focus=true

That is odd. The only difference is an additional line on Windows. I do not have a copy of Windows I can test on at this time, unfortunately. Mac/Linux, I have. Windows, I do not.

>
org.junit.ComparisonFailure: expected:<...ORTED CRASH DATA ===[
sequence_information:
 - sequence_name: Sequence: test
 - sequence_command: foo command
 - sequence_index: 0
 - sequence_commands: [null]
 - sequence_commands_descriptions: [foo command]
]
> but was:<...ORTED CRASH DATA ===[
sequence_information:
 - sequence_name: Sequence: test
 - sequence_command: foo command
 - sequence_index: 0
 - sequence_commands: [null]
 - sequence_commands_descriptions: [foo command]
<- This is the extra line
]

The code for the assert already adds two newlines.

        assertEquals("=== REPORTED CRASH DATA ===\n" +
                "sequence_information:\n" +
                " - sequence_name: Sequence: test\n" +
                " - sequence_command: foo command\n" +
                " - sequence_index: 0\n" +
                " - sequence_commands: [null]\n" +
                " - sequence_commands_descriptions: [foo command]\n" +
                "\n", stringWriter.toString());

Maybe, instead of doing an assertEquals, we can do an assertTrue:

        assertTrue(stringWriter.toString().contains("=== REPORTED CRASH DATA ===\n" +
                "sequence_information:\n" +
                " - sequence_name: Sequence: test\n" +
                " - sequence_command: foo command\n" +
                " - sequence_index: 0\n" +
                " - sequence_commands: [null]\n" +
                " - sequence_commands_descriptions: [foo command]\n" +
                "\n"));

Or strip both of newlines

        assertEquals(("=== REPORTED CRASH DATA ===\n" +
                "sequence_information:\n" +
                " - sequence_name: Sequence: test\n" +
                " - sequence_command: foo command\n" +
                " - sequence_index: 0\n" +
                " - sequence_commands: [null]\n" +
                " - sequence_commands_descriptions: [foo command]\n" +
                "\n").trim(), stringWriter.toString().trim());

@Stereo: If you have a convenient windows machine, can you check if the later passes? It should, but Windows is already exhibiting different behavior.

[taylor.smock]

Trim output on Windows (and other platforms) so that the content is checked, and not whitespace.

[Stereo]

I agree that it's very odd! This new github actions thing I'm working on is uncovering all kinds of silly little bugs that don't happen on every platform. Fun!

If you fork my github fork, it will automatically run the github actions for you. Trimming would make sense, I think?

[taylor.smock]

Replying to Stereo:

I agree that it's very odd! This new github actions thing I'm working on is uncovering all kinds of silly little bugs that don't happen on every platform. Fun!

If you fork my github fork, it will automatically run the github actions for you. Trimming would make sense, I think?

I've been trying really hard to avoid using Github for the past few years. Sometimes I have to use it for work, but all of my personal projects (and my projects that start out personal and then become something I work on at my job) are all on Gitlab. Even the stuff that I work on from Github I forked onto Gitlab and work on them there.

I'll see if I can figure out Github actions (I'd assume it isn't too difficult). I'll ping you on IRC if I encounter an issue (presumably you are Stereo on IRC).

As far as trimming goes, I don't know -- I didn't write the test, @simon04 did, and I don't know if the newlines are particularly important.

[taylor.smock]

Well, trim didn't work:

Test: testCreateReportedException took 376 milli sec(s) FAILED: expected:<...ORTED CRASH DATA ===[
sequence_information:
 - sequence_name: Sequence: test
 - sequence_command: foo command
 - sequence_index: 0
 - sequence_commands: [null]] 
 - sequence_command...> but was:<...ORTED CRASH DATA ===[
sequence_information:
 - sequence_name: Sequence: test
 - sequence_command: foo command
 - sequence_index: 0
 - sequence_commands: [null]  
]
 - sequence_command...>
org.junit.ComparisonFailure: expected:<...ORTED CRASH DATA ===[
sequence_information:
 - sequence_name: Sequence: test
 - sequence_command: foo command
 - sequence_index: 0
 - sequence_commands: [null]] 
 - sequence_command...> but was:<...ORTED CRASH DATA ===[
sequence_information:
 - sequence_name: Sequence: test
 - sequence_command: foo command
 - sequence_index: 0
 - sequence_commands: [null]  
]
 - sequence_command...>

It looks like a sequence_command isn't being added.

[Stereo]

CC'ing simon04. Hi, sorry I broke your test :)

[taylor.smock]

Use StringWriter/PrintWriter to produce the output to be checked. This ensures that the appropriate line endings are used for the platform.