Thanks again for the feedback. Getting back to this after working on other things for awhile. This patch continues fixing test related errors and failures. I've also rebased the other patches locally, will upload new versions of those. test_caldav_client I stubbed out the `checkRedirect` function to solve that issue. Then using the debugger (which required using `--flag icaljs` or `--flag libical` to run only a single test), I looked into the next failure with `getAllItems` returning an empty array. With the debugger running the test did not fail, and I could see that the async call to the database in `getItems` (here: https://searchfox.org/comm-central/source/calendar/providers/storage/calStorageCalendar.js#737) was returning a result, and the callback function was called. Without the debugger that async call to the database was resolving but without returning a result, and the callback wasn't getting called. It appears that the test is checking for something that is not there yet. Also, if I made two calls to `getAllItems` in the test, the first would return an empty array and the second would return the expected result. So adding `executeSoon` got the test to pass. (And also eliminated that warning about QueryInterface being undefined that was apparently related.) test_freebusy_request Given what you wrote on this, I took a new approach and changed the input to be correct -- DATE-TIME rather than just DATE. (Maybe we want to test incorrect input too, eventually but first things first.) That solved the problem where ical.js was expecting a DATE-TIME but was getting a DATE. However, ical.js and libical produce different results for the last `deepEqual` in the test given the same input, so one or the other will fail. Do we want to tweak ical.js output to match libical? Or...? test_freebusy_request - too much recursion issue I looked into this briefly, but haven't gotten to the bottom of it. Will keep working on it (and the other TODOs that aren't test related).
Bug 1546606 Comment 20 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Thanks again for the feedback. Getting back to this after working on other things for awhile. This patch continues fixing test related errors and failures. I've also rebased the other patches locally, will upload new versions of those. test_caldav_client I stubbed out the `checkRedirect` function correctly to solve that issue. Then using the debugger (which required using `--flag icaljs` or `--flag libical` to run only a single test), I looked into the next failure with `getAllItems` returning an empty array. With the debugger running the test did not fail, and I could see that the async call to the database in `getItems` (here: https://searchfox.org/comm-central/source/calendar/providers/storage/calStorageCalendar.js#737) was returning a result, and the callback function was called. Without the debugger that async call to the database was resolving but without returning a result, and the callback wasn't getting called. It appears that the test is checking for something that is not there yet. Also, if I made two calls to `getAllItems` in the test, the first would return an empty array and the second would return the expected result. So adding `executeSoon` got the test to pass. (And also eliminated that warning about QueryInterface being undefined that was apparently related.) test_freebusy_request Given what you wrote on this, I took a new approach and changed the input to be correct -- DATE-TIME rather than just DATE. (Maybe we want to test incorrect input too, eventually but first things first.) That solved the problem where ical.js was expecting a DATE-TIME but was getting a DATE. However, ical.js and libical produce different results for the last `deepEqual` in the test given the same input, so one or the other will fail. Do we want to tweak ical.js output to match libical? Or...? test_freebusy_request - too much recursion issue I looked into this briefly, but haven't gotten to the bottom of it. Will keep working on it (and the other TODOs that aren't test related).
Thanks again for the feedback. Getting back to this after working on other things for awhile. This patch continues fixing test related errors and failures. I've also rebased the other patches locally, will upload new versions of those. test_caldav_client I stubbed out the `checkRedirect` function correctly to solve that issue. Then using the debugger (which required using `--flag icaljs` or `--flag libical` to run only a single test), I looked into the next failure with `getAllItems` returning an empty array. With the debugger running the test did not fail, and I could see that the async call to the database in `getItems` (here: https://searchfox.org/comm-central/source/calendar/providers/storage/calStorageCalendar.js#737) was returning a result, and the callback function was called. Without the debugger that async call to the database was resolving but without returning a result, and the callback wasn't getting called. It appears that the test was checking for something that is not there yet. Also, if I made two calls to `getAllItems` in the test, the first would return an empty array and the second would return the expected result. So adding `executeSoon` got the test to pass. (And also eliminated that warning about QueryInterface being undefined that was apparently related.) test_freebusy_request Given what you wrote on this, I took a new approach and changed the input to be correct -- DATE-TIME rather than just DATE. (Maybe we want to test incorrect input too, eventually but first things first.) That solved the problem where ical.js was expecting a DATE-TIME but was getting a DATE. However, ical.js and libical produce different results for the last `deepEqual` in the test given the same input, so one or the other will fail. Do we want to tweak ical.js output to match libical? Or...? test_freebusy_request - too much recursion issue I looked into this briefly, but haven't gotten to the bottom of it. Will keep working on it (and the other TODOs that aren't test related).