Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Refactored tests building/passing, time zone bugs fixed, htmlBody bug fixed #42

Closed
wants to merge 16 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

scottnonnenberg commented Sep 14, 2012

Three major categories of changes in this pull request:

  1. tests are now building and passing, and hardcoded paths and credentials have been eliminated Details:
  • Build failure fixed - bodyPreferringPlainText now required a *BOOL parameter, so passing it in and checking the result in each CTCoreFolderTest test
  • New account.plist added to allow user to provide their own account information for testing
  • Hardcoded paths removed - tests now rely on relative paths
  • TestData added to project
  1. Timezone bugs in CTCoreMessage
    All timezone-related manipulation in CTCoreMessage has been removed, as NSDates do not have time zones. Timezones are a figment of an NSDateFormatter's imagination, and therefore sentDateGMT, senderDate and setDateLocalTimeZone all now return the same NSDate.

  2. htmlBody now calls fetchBodyStructure if needed
    The body accessor for plain text checks to see if myFields or myParsedMime are null, and if so calls fetchBodyStructure. Copied this code into htmlBody. Also added a convenience method useful to those who want HTML first: bodyPreferringHTML.

Misc:

  • files in the TestData directory are included in the project. This makes it a bit easier to work with them while writing and debugging tests. They are not included in any build target, nor is the new account.plist in the same directory.
  • README.md has been updated to give some information on how to run MailCore tests.
  • .gitignore updated to exclude user's account.plist, which will contain their email account's credentials
  • added a new test file with no plain text element, and two tests using it - one CTCoreMessage test, and one CTMIMETest

Kav Latiolais and others added some commits Aug 29, 2012

Kav Latiolais Fixed a typo 4d2d4ba
Kav Latiolais seriously... dadc7b8
Kav Latiolais Merge branch 'master' of github.com:Liffft/MailCore into HEAD fe25249
@scottnonnenberg scottnonnenberg Test building and passing
Build failure fixed - bodyPreferringPlainText now required a *BOOL parameter, so passing it in and checking the result in each CTCoreFolderTest test.
New account.plist added to allow user to provide their own account information for testing.
Hardcoded paths removed - tests now rely on relative paths.
TestData added to project.
Timezone manipulation removed, as NSDates do not have time zones. Timezones are a figment of an NSDateFormatter's imagination, and therefore sentDateGMT, senderDate and setDateLocalTimeZone all now return the same NSDate.
f57fcd3
@scottnonnenberg scottnonnenberg Merge branch 'master' of https://github.com/Liffft/MailCore
Conflicts:
[fixed]	MailCore.xcodeproj/project.pbxproj
d9b9e05
@scottnonnenberg scottnonnenberg Tweak: Fixed issue with project file merge 88145d7
@scottnonnenberg scottnonnenberg No more account.plist in repo
Now when developers create an account.plist on their own machine, that info won't get added back into the repo.
0722625
@scottnonnenberg scottnonnenberg Additional project file cleanup
When testing the project outside its parent projects, things didn't work so well. Removing OCMock stuff.

Also removing all TestData from build targets, and some nonexistent paths from library search paths.
54b8bbf
@scottnonnenberg scottnonnenberg htmlBody now fetches before building up string
It just seems to make sense that (void)htmlBody would have the exact same structure as (void)body except for the name of the function called inside. And now a new test passes.

Also: a bit of noise was coming from CTMimeTests/testBruteForce - commented out the NSLog.
1505574
@scottnonnenberg scottnonnenberg New bodyPreferringHTML convenience method b7282a4
@scottnonnenberg scottnonnenberg Readme edits fabff38
@scottnonnenberg scottnonnenberg Adding example and tests for no-plain text message
Added both a MIME test an a Message test for a message with no plain text body.

Also:
 - removed unnecessary NSLogs
 - removed unnecessary html-body example.
79ef5ad
@scottnonnenberg scottnonnenberg Merge pull request #1 from NiKnight/master
CTCoreMessage.htmlBody now fetches body first
7d080e6
@scottnonnenberg scottnonnenberg Tweaks for Pull request 54bfca6
@scottnonnenberg scottnonnenberg Merge remote-tracking branch 'upstream/master' b80f180
@scottnonnenberg scottnonnenberg Removing code-signing stuff for Pull Request de10811
Member

jwilling commented Sep 15, 2012

I must say I completely disagree with the date formatting. Personally I think the methods should remain, but if they return the same date all but one should be removed. Otherwise the method names are misleading and redundant.

Owner

mronge commented Sep 18, 2012

Awesome pull request! Thanks for your hard work

The only nitpick I have is around dates. I agree that the date code should be simplified, however the problem we run into is email dates are in the senders local time and not GMT.

Here is from the spec

" The date and time-of-day SHOULD express local time.

The zone specifies the offset from Coordinated Universal Time (UTC,
formerly referred to as "Greenwich Mean Time") that the date and
time-of-day represent. The "+" or "-" indicates whether the
time-of-day is ahead of (i.e., east of) or behind (i.e., west of)
Universal Time. The first two digits indicate the number of hours
difference from Universal Time, and the last two digits indicate the
number of minutes difference from Universal Time. (Hence, +hhmm
means +(hh * 60 + mm) minutes, and -hhmm means -(hh * 60 + mm)
minutes). The form "+0000" SHOULD be used to indicate a time zone at
Universal Time. Though "-0000" also indicates Universal Time, it is
used to indicate that the time was generated on a system that may be
in a local time zone other than Universal Time and therefore
indicates that the date-time contains no information about the local
time zone."

http://tools.ietf.org/html/rfc2822#section-3.3

So instead we should probably keep just sentDateGMT and remove sentDateLocalTimeZone. The other methods senderDate and senderTimeZone would then become internal methods.

What do you think?

Contributor

jdg commented Sep 18, 2012

+1

Owner

mronge commented Sep 21, 2012

I tried running this but the relative paths in the tests don't work for me, it can't find the files.

I opened up the project and switched to iPhone 6.0 Simulator then ran Test. I also did setup an account.plist file

Is there anything else I need to change so that the tests find my files?

Contributor

scottnonnenberg commented Sep 21, 2012

Hey guys. I'm definitely running into some issues as well after an XCode upgrade. Look for some updates to this pull request soon - both to address that and to address some of the timezone confusion. Specifically, I plan to eliminate all but senderDate and include a specific test to show that two emails sent from different time zones (but equivalent times) actually result in equivalent NSDate objects.

Owner

mronge commented Oct 8, 2012

Did you ever figure out what was going on with Xcode and the tests? I would love love love to include this pull request once we sort that out

Contributor

scottnonnenberg commented Oct 11, 2012

Unfortunately not yet. I've been deep in other high-pri dev activities for my company. Should be able to get to this next week.

On Oct 8, 2012, at 3:03 PM, Matt Ronge notifications@github.com wrote:

Did you ever figure out what was going on with Xcode and the tests? I would love love love to include this pull request once we sort that out


Reply to this email directly or view it on GitHub.

alloy commented Dec 19, 2012

I would love to merge the test fixes/updates into my fork.

@mronge If I understand it correctly, I should be able to merge the test fixes/updates except for the date stuff?

Owner

mronge commented Dec 19, 2012

Yes, something similar to those date fixes have been merged in.

There are also some changes made so that it uses a relative path for test data but it seems to have broken in the latest Xcode.

On Dec 19, 2012, at 9:14 AM, Eloy Durán notifications@github.com wrote:

I would love to merge the test fixes/updates into my fork.

@mronge If I understand it correctly, I should be able to merge the test fixes/updates except for the date stuff?


Reply to this email directly or view it on GitHub.

Contributor

scottnonnenberg commented Dec 21, 2012

Hey guys. Unless anyone objects, I'm planning to close this in light of my new pull request.

@mronge mronge closed this Jan 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment