Modify

Opened 3 weeks ago

Closed 3 weeks ago

Last modified 2 weeks 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 3 weeks ago.
19407.windows_trim.patch (1.1 KB) - added by taylor.smock 2 weeks 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 13 days 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 3 weeks ago by taylor.smock

Attachment: 19407.patch added

comment:1 Changed 3 weeks 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 3 weeks ago by simon04

In 16682/josm:

see #19407 - Add SequenceCommandTest.testCreateReportedException

comment:3 Changed 3 weeks ago by simon04

Milestone: 20.06

comment:4 Changed 2 weeks 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 2 weeks 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 2 weeks 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 2 weeks 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 2 weeks 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 2 weeks 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 2 weeks ago by taylor.smock (previous) (diff)

comment:9 Changed 2 weeks ago by Stereo

Cc: simon04 added

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

Changed 13 days 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.