650963 - controller.assertText should be renamed to assertInnerHTML

Status: RESOLVED WONTFIX

(Testing Graveyard :: Mozmill, defect)

Description

[Andrew Halberstadt [:ahal]]

As M.A Darche mentioned in the newsgroup, assertText is confusing. This method doesn't check 'text', but rather the innerHTML of the element.  This is not at all what assertText implies.  I've gone ahead and updated the wiki to say that it checks innerHTML but I would take it one step further and rename the method to assertInnerHTML or something similar.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We should not rename those assert methods. We should completely remove them. It doesn't make sense anymore to keep them supported. We have general assert methods (see the new assertion module) and an element hierarchy which let us define to get the value or content of an element. All that makes those scary assert methods obsolete. Here an example:

Before: controller.assertValue(textbox, "Foo bar");
After:  expect.equal(textbox.value, "Foo bar");

Before: controller.assertText(div, "Foo bar");
After:  expect.equal(div.content, "Foo bar");

Comment 2

[cmtalbert]

I agree with Henrik, but it's too late to take them out cleanly.  Let's file a follow on to take the old assert methods out, and in the meantime, rename this one since that is a simple fix.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

In this case we should mark all of them as deprecated in Mozmill 2.0

Comment 4

[Andrew Halberstadt [:ahal]]

How large is the scope of the the new assert module?  When you say deprecate the old ones are we talking about all of them?  Including the general assert method and assertJSProperty etc.?

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

'assert' is way to complicated to use. So yes, it will die. Similar for assertJSProperty, but parts of that code we should move to the Element class, so we can have two functions:

Element.getJSProperty(aName)
Element.getDOMProperty(aName)

With those methods we can use the new assertion module as following:

expect.equal(elem.getJSProperty('visible'), true, "Element is visible");
assert.notOk(elem.getJSProperty('unknown'), "Property does not exist");

Comment 6

[cmtalbert]

So, what's the way forward here?

Deprecate all the old assertFOO methods?  That's a large change and one that I didn't think we were going to take for 2.0.

I like Henrik's idea in comment 5, it does make it clean.  We need a final decision here so it's clear what we want to implement for 2.0.

Comment 7

[cmtalbert]

The way forward: Add deprecation messages to all these assert funcitons.  We will not rename the function right now because it's going to go away.


Any element specific functions will also be marked for deprecation on controller but may be moved to the element class in a future version.  Please open a follow up bug on that. (see comment 5).

Comment 8

[Heather Arthur [:harth]]

We're deprecating the controller.assert functions in favor of the assert module see https://bugzilla.mozilla.org/show_bug.cgi?id=666008