Refactoring into cpan module #9
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moving most of the code into App::ICalToGCal (maybe want a better name?), so that the ical-to-gcal script is just a thin wrapper that calls that code. This means I can distribute this via CPAN, and also that I can write some proper tests for it.
Be a nice Internet citizen and identify ourselves.
Fetch our Net::Google::Calendar object via a class method that can also be used to replace it, so test scripts can provide a mock object that implements the required features, and we'll just use it.
A lot more to do, but committing this now. Currently prepares a mocked Net::Google::Calendar object, passes it to the gcal_obj class method, and tests that we then get that mocked object back when we ask for a Net::Google::Calendar object. Next up: tests that, when given known iCal data, the results stored in the mock object are as we'd expect.
Not yet complete - more hacking to do here - but handles looping over a set of test specs, each of which names an iCal data file to parse, and provides a hashref describing what the resulting entries added to the Google calendar ought to look like. Will need a flag in each test spec to indicate if the entries in the mock gcal object should be wiped before starting, so some tests can be testing only that the results from that parse are correct, but others can test that the results look as expected after making changes based on the previous one too (to e.g. test for events that are no longer in the ical feed / have been modified etc being handled correctly).
Unless the test says otherwise, wipe all events from the mock object before starting each test. That way, each test can easily compare the list of events to what it expects to see, and not see stuff from previous tests there - but tests which want to build upon previous results (e.g. testing that events no longer in the source iCal feed get deleted, updated events get updated, etc) can do so.
Also dump some excess debug output.
One less thing to do when this nears release.
Test::Differences, given a hash of scalars, uses the stupid, nasty, unhelpful flatten style, which is SHIT. See: https://rt.cpan.org/Public/Bug/Display.html?id=95446 I may have tie @DrHyde's beard to a chair tomorrow and refuse to release him unless he fixes that nastyness so this horrific bodge can be reverted.
We want to return DateTime objects using the timezone from the iCal feed, but we don't want to call set_time_zone() to set the timezone as I was, because that changes the time represented by the object too. This was is a bit clumsy but works. FIXME: it seems to be dog-slow - probably because it's calculating loads of DST changes etc. May have to revert this and implement my own parsing of the ISO8601 datetimes, and just create DT objects with the right datetime and timezone set? Or maybe Data::ICal has something useful.
The previous approach, calling set_time_zone(), was *HORRIBLY* slow due to all the daylight savings time calculations DT had to perform. It was leading to results like: ``` ``` ... because of all the objects DateTime was flinging about. This way works almost instantaneously. Tests still fail because of timezone fun - it would appear that the resulting timestamp we're seeing is that of the local timezone - may need to fake that in the tests so that we don't fail if the user running the tests is in a different timezone!
This reverts commit 1d957b5.
When we pass DateTime objects to Net::Google::Calendar::Entry's when() method, it automatically converts them to UTC; we want to test that the result is correct. Using a timezone that doesn't observe DST means the result will always be the same, rather than us failing our tests in the summer/winter :)
This is a bit hackish perhaps; might be better to fetch the raw dtstart/dtend and inspect for absence of a time component.
Swapping DateTime::Format::ISO8601 for DateTime::Format::DateParse. Nod to Pizentios on Freenode/#perl for the recommendation.
Want to be able to test that the [ical_imported_uid] tag is being set properly. FIXME: doesn't actually work - we get back some encoded nastyness instead.
I've decided to leave the code here for now, as fixing it might not be too difficult, I just don't have the time/motivation to do it at present as I no longer use it, but someone might want to contribute the required changes, or buy me a few beers to do it, whatever.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Work to refactor this into something CPAN-releasable and testable. This is turning out to be a fair amount of work, but will at least mean the module is more flexible and properly tested. A lot of work for something I don't really use myself any more though!