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.
Created attachment 151035 [details] Patch
CC'ing Adam and Kihong.
If there is a problem what you said, how about checking both value and type. IMHO, we need to check values which is returned. :)
(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.
(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.
Created attachment 151139 [details] Patch
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.
(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?
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.
(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.
Created attachment 151180 [details] Patch
Comment on attachment 151180 [details] Patch Clearing flags on attachment: 151180 Committed r122088: <http://trac.webkit.org/changeset/122088>
All reviewed patches have been landed. Closing bug.