Shawn Wilsher

Back in January the Download Manager broke due to a change in the JS engine. However, since the UI had no tests on it, the issue was not caught upon checkin. This led to a patch franticly being created changing nearly all instances of let to var. I was not happy about this since most of the changes were not necessary, and valuable cvs blame data was then lost on the file. Upon brining this up, I got blasted for not having any tests on the UI when I’m clearly supposed to have them as per the toolkit test requirement.

I’m not saying that getting blasted was uncalled for. In fact, it wasn’t. It was more of a wake-up call that I was being an irresponsible owner of my submodule to me. Test driven development is really an awesome model, and it had saved me many times before with the back-end of the download manager (which did/does have fairly extensive unit tests). Having said that, you’d think I would have known better. Maybe it was laziness (writing a test takes a while), or maybe it was lack of knowledge (I only knew about xpcshell unit tests at the time), but really, it was inexcusable.

I’d like to say that I’ve learned my lesson now. While the download manager UI isn’t in a state that I’d consider “well tested” yet, it’s leaps and bounds better than it once was. After I wrote the first test case of the UI, things got a lot easier. Since then, Edward has landed several tests cases (as well as putting up a patch to improve the reliability of our tests), and a few other contributors have added some test cases.

It’s not all fun in the sun now that we have tests though. Based on conversations I’ve had on irc, or quotes that I’ve seen go by on qbo, it seems quite apparent to me that I’ve joined the ranks of those who are jerks about test cases. It’s not that I don’t mind that either – I think it’s the right approach.

However, I’ve noticed a steady decline in morale from contributors to my module (and community members attitudes toward me as well). Two bugs, in particular, come to mind. With the first case, I’ve gone so far as to explain exactly what should be tested, although maybe in not enough detail. I’ve seen comments go by on irc that what I’m asking for is “unreasonable”, especially because it was just a typo that caused the regression. Sadly, I fear that the people advocating this just don’t understand the test-driven development model. Clearly this was something that can be easily regressed if just a minor typo caused it, so it should have a test to make sure something as trivial as a typo doesn’t cause it again. I mean, it only took us a few days to track the issue down to this bug, and figure out what was wrong. Another issue brought up with that bug is that the test would be “unreliable”. To me, that’s just an excuse to not write the test though – I don’t see how it’s unreliable in any way. I’m sure that was a great use of everyone’s time, and that they’d all love to do it again in the future. Or, we can have it tested and never have to worry about it again. The second case isn’t nearly as controversial, but I fear that I’ve scared a long-time contributor away simple because he wanted to change the behavior of untested code that I want tests on before it’s changed.

You might notice that I sounded a bit angry in that last paragraph. I have to say that it’s quite frustrating to see so much resistance to the test driven model of development. It’s proven to work in lots of projects. Sure, writing tests can often take much longer than fixing the bug (it took me about twice as long to get the initial unit tests working for the download manager back-end than it did to do the rewrite), but the amount of time it saves you down the line is immeasurable. The sooner people realize this, the better off our entire codebase will be. Yeah, I’ve been making the problem worse by getting more and more strict about the test requirement. However, it’s the right thing to do.

Reflecting on this has brought up at least one issue. How do we require tests without scaring contributors away? I rather like how bsmedberg asked me to add a test for nsIMutableArray the other day on irc by suggesting that if I depended on a behavior, I add a test for that behavior. This doesn’t really work in my submodule though since the tests are required for the code to land. It’s an issue that should probably be looked into.

EDIT: I didn’t post this last night because in the back of my mind I knew my tone was too harsh, so I slept on it. After waking up, fixing it, and reading a particular post on planet, I see that I was in fact correct about there being frustrations with contributors. It’s unfortunate, and I don’t really know how to fix the problem yet.

I figure I should spruce this negative entry up by citing two cases where random outside contributors came through with great tests without hesitation – bug 333848 by Lev Serebryakov, and bug 403068 by John Zhang from Aptana.

So, as many people running nightlies may have found out, the version of the DOM Inspector that they got from AMO didn’t work. I resolved that in bug 420047 earlier today. That means that as of tomorrow’s nightly, DOMi should work again. That’s right – it wasn’t a problem with the xpi, rather we were a bit too aggressive in removing files (namely, inspector.xpt) when DOMi was disabled by default.

Any other issues found should be filed as new bugs. I’ve opened up a meta bug to track the release of 2.0.1 of the DOM Inspector.

I just resolved bug 271812, which means you can now get the DOM Inspector for 1.9 products off of AMO. Nightly testers (and future beta 4 users) will notice that it’s no longer included by default in the product. You can get the add-on here! The UI is no longer coupled to the release of Firefox, so we can update things independent of the Firefox release cycle. Certain back-end changes will still depend on the Gecko release schedule, but I think I’m OK with that since that would require shipping binary components if we didn’t want to do that. Binary components are hard, and other add-ons (like Firebug) depend on some of them too. It’s best if they stay a part of layout for the time being.

I recently fixed Bug 416173, which changes how database connections are opened for mozStorage. The important thing to know is that they don’t throw if the database is corrupted or couldn’t be opened for whatever reason. You have to check the connectionReady property that’s always been there to know if it’s safe to use or not. I’ve filed bugs for components that I knew used the storage service, but I’m betting there are consumers out there on 1.9 that I don’t know about. This post is me telling you to file bugs to get those consumers fixed! Please mark those bugs dependent on 416173 too!

You’ll note that this was fixed roughly two weeks ago. That’s a good demonstration on just how busy I’ve been and why I’ve been essentially non-existent on irc. I cannot wait for school to be over!

That’s right! Last night I landed the latest version of SQLite to trunk. While generally updating SQLite is not so new-worthy, this time it is. You see, SQLite 3.5.x brings about some really awesome changes that consumers can take advantage of. The most important one for Mozilla (as I see it so far) is the ability to have the shared cache accessible on multiple threads. That means we can use database connections on multiple threads now while using the same cache. We can also use them at the same time (before, you could use them on different threads, but they would not have the same shared cache, nor could you use them at the same time). To see all the neat changes that we can now use, please read the SQLite docs.