Tuesday, December 17, 2013

Resetting database state for functional testing

Use mysqlhotcopy and just copy the files back.

It's a testing server.  We know nothing's happening except the tests.  So (for MyISAM) we can just copy the files and flush tables.

Using a mysqldump style restore was taking ~40 secs.

Copying files is taking ~1.5 seconds.  And I suspect most of that is the time spent by phing and to establish an ssh connection - the actual copying directly on the server takes no time at all.

This won't work like this for InnoDB (when we get to that point).  I'll need, I think, either to work out a filesystem (LVM) snapshot approach, or use Percona Xtrabackup.


Monday, December 9, 2013

Security smells

Time to coin a term.

A 'security smell' is an indication that something's wrong in a system that you're using from a security perspective.  The are worse than code smells, because sniffing a security smell very likely means that something's actually wrong, rather than 'could be better'.  Security smells are things you notice just by using the system, not by actively trying to find them, or from looking at code.

Security smells I've thought of so far:
  1. 'Forgotten password' emails you your actual password.  That means it's not hashed in the backend storage.  That's bad.
  2. System error messages.  Oops
  3. Careless information leaking.  For example, I sign up to a demo and see the names/emails/whatever of others that have done so.
  4. Broken character sets/encoding.  A special character comes back mangled.

Friday, December 6, 2013

Selenium / phpunit gotchas

Some notes to self...


- IEdriver (at least IE10 in Win7 so far) doesn't click on non-form elements that are (wholly) in the viewport.  Need to $this->move($element); where $element is something that means that the target you want to click is fully in the viewport.  Not sure at the mo about links but they might be OK.

- click()ing on an <a> (in IE11) didn't work; click()ing on a child node did.  Don't know why.

(Update 17/12/2013)
 - Colons in css id selectors break ('An invalid or illegal string was specified').  So instead of $this->byCss("#elem:a1"); it needs to be $this->byCss("#elem\:al");

Wednesday, December 4, 2013

YUI2 Rich text editor and IE11


Doesn't work so much.  Editor uses a (slightly modified) version of the YAHOO.env.ua object internally.  This doesn't recognise IE11 because they've got rid of the MSIE string in the user agent.

The patch is in the browser property of editor:

+            if (navigator.userAgent.indexOf('Trident/7') !== -1) {
+                //rv:11.0)
+                try {
+                    br.ie = parseFloat(navigator.userAgent.match(/rv:([\d\.]+)\
+                } catch (e) {}
+
+            }



Trident is the 'IE', rv:11.0 is the version.

It all seems to work again after this.  The UA sniffs that were needed before still appear to be required.

Goodbye, afternoon, tracking that down and checking it all...

Update

OK, not quite.  That was part of it, but there's some other goodies too.  The MS changes page here lists some of the changes, though not sure it's all of them.

Firstly, selection has changed (http://msdn.microsoft.com/en-us/library/ie/ff975315(v=vs.85).aspx) - it now implements IHTMLSelection. In editor's getRange method there's a check for IE:

            if (this.browser.ie) {
                try {
                    return sel.createRange();
                } catch (e2) {
                    return null;
                }
            }

            if (sel.rangeCount > 0) {
                return sel.getRangeAt(0);
            }
            return null;

createRange() doesn't exist in IE11, but sel.rangeCount and sel.getRangeAt() do, so we can patch with:

            if (this.browser.ie && this.browser.ie < 11) {
                try {
                    return sel.createRange();
                } catch (e2) {
                    return null;
                }
            }

            if (sel.rangeCount > 0) {
                return sel.getRangeAt(0);
            }
            return null;


Secondly, something about readystate has changed, although I'm not quite clear how this works it's way through to the editor exactly, but it's a problem.  In Editor's _checkLoaded() method, we have:

            try {
                if (this._getDoc() && this._getDoc().body) {
                    if (this.browser.ie) {
                        if (this._getDoc().body.readyState == 'complete') {
                            init = true;
                        }
                    } else {
                        if (this._getDoc().body._rteLoaded === true) {
                            init = true;
                        }
                    }
                }
            }

But this._getDoc().body.readyState never seems to happen, (while _rteLoaded does), so the Editor never gets init-ed.  Again, changing this to

            try {
                if (this._getDoc() && this._getDoc().body) {
                    if (this.browser.ie && this.browser.ie < 11) {
                        if (this._getDoc().body.readyState == 'complete') {
                            init = true;
                        }
                    } else {
                        if (this._getDoc().body._rteLoaded === true) {
                            init = true;
                        }
                    }
                }
            }

(note the version number) seems to fix it.

Finally, it seems like window.event has gone, which breaks _getSelectedElement().  Again, changing

        _getSelectedElement: function() {
            var doc = this._getDoc(),
                range = null,
                sel = null,
                elm = null,
                check = true;

            if (this.browser.ie) {
                this.currentEvent = this._getWindow().event; //Event utility assumes window.event, so we need to reset it to this._getWindow().event;
                range = this._getRange();
to


        _getSelectedElement: function() {
            var doc = this._getDoc(),
                range = null,
                sel = null,
                elm = null,
                check = true;

            if (this.browser.ie && this.browser.ie < 11) {
                this.currentEvent = this._getWindow().event; //Event utility assumes window.event, so we need to reset it to this._getWindow().event;
                range = this._getRange();


fixes it for us.


Incidentally, I also moved the original patch at the top of this page out of Editor's browser property to update YAHOO.env.ua:

if (YAHOO.env.ua.ie === 0 && navigator.userAgent.indexOf('Trident/7') !== -1) {
    // version is in a string like this: rv:11.0)
    try {
        YAHOO.env.ua.ie = parseFloat(navigator.userAgent.match(/rv:([\d\.]+)\)/)[1]);
    } catch (e) {}
 }

Writing Selenium tests with sausage (and phpunit).

This is thinking aloud as I write some tests...;

API things I'd like to fix:



in sausage:


spinAssert() doesn't show you what went wrong if you have assertions inside the callback function.  Although the assertions (appear to) get called, if one fails (not just because the page isn't ready, but because of an error) you don't know which/why.  You just get `failed asserting that false is true` (which is the return value of the callback).

in phpunit



$parentElement = $this->byCss('div');
$childElement = $this->byCssSelector('span');
// Really?  byCssSelector ?  Is there a reason not to be byCss()?

// and then:
$moreChildren = $this->elements($this->using('css selector')->value('.ugh'));

// so now there's using('css selector')->value('') ?
// how about
$this->allByCss('.ugh');

// or better, as convenience methods:
$oneElement = $this->one('div');
$anotherEl = $this->one('#eleid div.whatever span');
$lotsOfElements = $this->all('#eleid div');
// and always use css selector




Overall structure of tests


This is a question.  The advice is that there should be no dependencies between tests.  That definitely makes sense with unit testing where you're mocking things and don't (want to) touch the database.

But functional/user acceptance tests (what I'm writing) do touch the database, that's the point of them.  I want to test that things save and show up again.   So say I have one test that counts the number of records, and another that creates a new record.  Then there's a dependency between them: the order of the tests matters, unless I reset the database before every test.

But I don't want to do that.  It takes almost a minute.  I can't do that in-between every test.

So I'm thinking that dependencies between tests isn't a bad thing, although it's probably better that dependencies only exist within one class (ie no dependencies between test classes).

Second question.  It seems that a page load is inevitable before each test.  This isn't how our application is used - generally it's a one page load and then ajax everything.  Initial page load isn't optimised particularly after login, because it only happens once* a day.  So keeping things clean, with lots of small tests, each testing one thing, is 1. slow (because of page load) and 2. unreal - it's not how the application is actually used.  I want my tests to be fast and realistic.  So now each test is getting really big - 20+ assertions and several spinAsserts() (and nested spinAsserts!).

Perhaps I should refactor each test along the lines of

public function testEditARecord() {
    $this->_loadTheRightPage();
    $this->_enterSomeValues();
    $this->_saveIt();
    $this->_checkValuesShownAreTheOnesIEntered();
}


These might have a small amount of value in terms of re-usability; but the point is making things a bit more readable.