For some time now, I have suspected that the asynchronous nature of Dart's unit tests have been causing problems in the test suite for ICE Code Editor. In fact, I have implemented a workaround more than once, only to remove it when I found an alternate approach.
During last night's #pairwithme session with Srdjan Pejic, we hit a problem directly caused by it. We eventually found an unsatisfactory workaround, but enough is enough. It is time to improve the asynchronous support in ICE.
Last night's problem occurred in the new Download feature. In the teardown for the group of tests that verify the feature, we created a new instance of the localStorage
Store
class so that we can clear all entries: group("Download", () {
tearDown(() {
// ...
new Store().clear();
});
// tests down here
});
We have two tests for the download feature—one that verifies the download functionality and the other that simply checks that clicking the Download link closes the menu. We did not struggle with the actual implementation, but rather with the tests. And the point of struggle caught us off guard. It was not the implementation of the features, but rather the setup. In both tests, we created a project named “Downloadable one”: test("it downloads the source as a file", (){
// ...
helpers.createProject("Downloadable one");
// ...
});
test("closes the main menu", () {
// ...
helpers.createProject("Downloadable one");
// ...
});
Since the tearDown()
function clears the data store between runs, we did not think too much of this... until the second test failed when creating its project. The reason the second failed was that the two tests share a common data store. The teardown for both is executed after the test run, but since both tests are run in parallel, both tests try to create the same project at around the same time—and well before the teardown would clear test data.This is confirmed if I put a
print()
statement at the top of the second test describing the contents of the data store: test("closes the main menu", () {
var editor = new Full(enable_javascript_mode: false);
_test(_) {
print(new Store().keys.toList());
helpers.createProject("Downloadable one");
// ...
}
});
The result is that there is already a “Downloadable one” project before we try to create a new project of the same name:unittest-suite-wait-for-done undefined:1 [Downloadable one, Untitled]We eventually worked around the problem by creating differently named projects, but that is no permanent solution. Instead I am going to introduce the option to set the store's key value in localStorage.
By default, this value needs to be
codeeditor
(a holdover from the JavaScript version). This is a static property of the Store
class that is used whenever data is written to localStorage. To optionally set the storage key, I introduce a new storage_key
instance variable that the constructor defaults to codeEditor
:class Store implements HashMap<String, HashMap> { /// The key used to identify the data in localStorage. String storage_key; /// The default key used to identify the data in localStorage. static const String codeEditor = 'codeeditor'; Store({this.storage_key: codeEditor}); // .... void _sync() { window.localStorage[storage_key] = JSON.stringify(projects); _syncController.add(true); } }Then, whenever there are writes to localStorage, such as in the
sync()
method, I use this new storage_key
value. If not specified in the constructor (which is how every current instance is created), then nothing changes.But in my download test, I can explicitly set the value of the
storage_key
property: setUp((){
editor = new Full(enable_javascript_mode: false)
..store.storage_key = "ice-test-${currentTestCase.id}";
});
Here, I have moved the editor assingment into a common setup now that I have better async support. I assign the editor variable to a new instance of the full-screen editor. With a method cascade (returns the value of the object receiving the method call, not the value of the method), I set the associated storage key to a unique value—a string that includes the current test case's ID.With that, I can again run my download tests using the same project titles:
group("Download", () {
var editor;
setUp((){
editor = new Full(enable_javascript_mode: false)
..store.storage_key = "ice-test-${currentTestCase.id}";
});
// ...
test("it downloads the source as a file", (){
helpers.createProject("Downloadable one");
// ...
});
test("closes the main menu", () {
helpers.createProject("Downloadable one");
// ...
});
});
And that does the trick. The two tests can run in parallel, both writing to localStorage, but now to two different locations in localStorage.In the end, I switch back to two different project titles—it seems more readable. The point of this was not so much to write the tests with the same names. Rather, it was to ensure that we have a way to avoid similar confusion in the future.
Day #781
No comments:
Post a Comment