Code review comment for lp:~dooferlad/launchpad/upcomingwork-expand-all

Revision history for this message
Curtis Hovey (sinzui) wrote :

You can ignore the random numbers. YUI assigns a unique id to every node it creates.

The test-suite should only test the functions in the code you wrote. We want to verify that the correct function is wired to the correct element. Since the functions are anonymous, I imagine we want to test that the element has or does not have the "hidden" class.

According to the module you created, it only requires lp.app.widgets.expander, so all but the last dependency can be deleted from the html file.

The html file is not setup to be reset by the test. Take a look at
    lib/lp/code/javascript/tests/test_util.*
Which uses the 'type="text/x-template"' technique to create html fragments for tests. The setup and teardown in the test-suite updates the fixture node to ensure each test has pristine markup.

test_util.js is testing several setup functions that also do hide and show (but not expanders). You might find this test helpful because it illustrates how to verify a method was called during an event, then checks the final state of the element. I think this style of test could be adapted by passing in an example expander config.

« Back to merge proposal