Bug 90665 – Check returned value in BatteryStatus APIs test cases

• Component: New Bugs
• Status: RESOLVED FIXED

[Gyuyoung Kim]

Test cases of battery status have checked if properties of battery status are existed. In my opinion, to check return type is better than to check properties.

[Gyuyoung Kim]

Created attachment 151035 [details]
Patch

[Gyuyoung Kim]

CC'ing Adam and Kihong.

[Kihong Kwon]

If there is a problem what you said, how about checking both value and type.
IMHO, we need to check values which is returned. :)

[Gyuyoung Kim]

(In reply to comment #3)
> If there is a problem what you said, how about checking both value and type.
> IMHO, we need to check values which is returned. :)

Can we estimate what is return values ? In network information APIs case, I just checked return type because returned value can be different on each port.

[Kihong Kwon]

(In reply to comment #4)
> (In reply to comment #3)
> > If there is a problem what you said, how about checking both value and type.
> > IMHO, we need to check values which is returned. :)
> 
> Can we estimate what is return values ? In network information APIs case, I just checked return type because returned value can be different on each port.

That's why we use internals.setBatteryStatus.
Every test cases set values for checking return values by 
internals.setBatteryStatus.

-------------------------------------------------------------------------------
var charging = false;
var chargingTime = Number.POSITIVE_INFINITY;
var dischargingTime = 6000;
var level = 0.7;
...
internals.setBatteryStatus(document, 'chargingchange', charging, chargingTime, dischargingTime, level);
-------------------------------------------------------------------------------

When internals.setBatteryStatus called, DRT emulates event-dispatching with preset values.

[Gyuyoung Kim]

Created attachment 151139 [details]
Patch

[Gyuyoung Kim]

I'm not sure whether each port can support this test cases in future. If possible, I also think to compare returned value is more correct. Patch is updated.

[Kihong Kwon]

(In reply to comment #7)
> I'm not sure whether each port can support this test cases in future. If possible, I also think to compare returned value is more correct. Patch is updated.

I think new patch is not different from original test cases.
It just changed from variables to values.
I looked into shouldBe function again, IMHO we don't need to fix these test cases.
How do you think about this?

[Adam Barth]

IMHO, this approach is better because the expected.txt file lists the actual values we expect.  That means we don't need to refer to the test to understand what is different about failing values.

[Gyuyoung Kim]

(In reply to comment #9)
> IMHO, this approach is better because the expected.txt file lists the actual values we expect.  That means we don't need to refer to the test to understand what is different about failing values.

I also agree with your thought. It is better to compare concrete values to recognize what is differences.

[Gyuyoung Kim]

Created attachment 151180 [details]
Patch

[WebKit Review Bot]

Comment on attachment 151180 [details]
Patch

Clearing flags on attachment: 151180

Committed r122088: <http://trac.webkit.org/changeset/122088>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.