Modify

Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#19407 closed enhancement (fixed)

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

Reported by: taylor.smock Owned by: team
Priority: normal Milestone: 20.06
Component: Core Version:
Keywords: Cc: simon04

Description

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.

Attachments (3)

19407.patch (2.6 KB) - added by taylor.smock 12 months ago.
19407.windows_trim.patch (1.1 KB) - added by taylor.smock 12 months ago.
Trim output on Windows (and other platforms) so that the content is checked, and not whitespace.
19407.printwriter.patch (1.7 KB) - added by taylor.smock 12 months ago.
Use StringWriter/PrintWriter to produce the output to be checked. This ensures that the appropriate line endings are used for the platform.

Download all attachments as: .zip

Change History (12)

Changed 12 months ago by taylor.smock

Attachment: 19407.patch added

comment:1 Changed 12 months ago by simon04

Resolution: fixed
Status: newclosed

In 16681/josm:

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

comment:2 Changed 12 months ago by simon04

In 16682/josm:

see #19407 - Add SequenceCommandTest.testCreateReportedException

comment:3 Changed 12 months ago by simon04

Milestone: 20.06

comment:4 Changed 12 months ago by Stereo

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

comment:5 in reply to:  4 Changed 12 months ago by 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.

Changed 12 months ago by taylor.smock

Attachment: 19407.windows_trim.patch added

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

comment:6 Changed 12 months ago by 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?

comment:7 in reply to:  6 Changed 12 months ago by 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.

comment:8 Changed 12 months ago by 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.

Last edited 12 months ago by taylor.smock (previous) (diff)

comment:9 Changed 12 months ago by Stereo

Cc: simon04 added

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

Changed 12 months ago by taylor.smock

Attachment: 19407.printwriter.patch added

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.