Opened 7 years ago

Closed 7 years ago

#16001 closed defect (patchwelcome)

doh _browserRunner.js - add id's to error/failure report

Reported by: lee Owned by:
Priority: undecided Milestone: tbd
Component: TestFramework Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

We're using selenium for CI testing (junit) using the doh runner and I'd like to be able to easily query the response body for errors/failures to set the assertion result.

I've attached a "patch" which is just a hack of the current code though it works for my use case. It's difficult to test when the logBody logs the actual results in sendToLogPane() because it uses the arguments object which is expected to be be used as a variable length array of strings to be logged.
It's called from runner.js for all runner's and I'd need to be careful I don't upset the ordering?

I added id's to the divs based on the string value of args[2] if it equals "errors" or "failures". It's flaky but if you want me to provide a a less brittle solution I can have a look, though the doh runner code is a bit unwieldy.

Attachments (1)

browserRunnerReportIds.diff (575 bytes) - added by lee 7 years ago.
patch to add id's to the error/failures report (apologies, bad diff attached)

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by lee

Attachment: browserRunnerReportIds.diff added

patch to add id's to the error/failures report (apologies, bad diff attached)

comment:1 Changed 7 years ago by lee

Can someone look at this please? It does seem like a very low risk change

comment:2 Changed 7 years ago by bill

Looks like you are talking about the summary section, listing the number of errors and successes, as opposed to lines listing each individual error/failure. Otherwise you'd be looking at args[0] rather than args[2].

Like you said, your patch is a hack to workaround the fact that the this.debug() calls scattered throughout DOH have no semantic information, not even a class name or enumerated id about the type of message. I agree the current system is a mess, so I see why you hacked in the change. But I'd rather check in a proper feature than a hack like this.

comment:3 Changed 7 years ago by lee

I'm happy to properly refine the patch so it's usable, though it seems like doh has suffered from inactivity for some time so it worries me this is just "putting a plaster on it".

It's messy to work with, I don't want to break anything so I'd need some feedback, I'll provide tests if possible (doh testing doh!)

comment:4 Changed 7 years ago by bill

Yah, unfortunately that's the case; I don't quite consider myself a DOH expert but I can give feedback on a patch. Not sure what is best, perhaps if the first argument to doh.debug() is a (non-string) Object that it uses it for metadata?

Another annoyance is that DOH is setup to work in different environments (besides the browser, node is the other important one), so need to keep all of those working.

comment:5 Changed 7 years ago by bill

Resolution: patchwelcome
Status: newclosed

OK, so if you can give a different patch, like where you call different functions (instead of this.debug()) depending on the type of message, or have a general function but pass some metadata to it, then I'd be happy to look at it.

Note: See TracTickets for help on using tickets.