justicenation / slashclock

/clock concept as a new Slack app
GNU General Public License v3.0
0 stars 0 forks source link

Regression tests for America/Los_Angeles time zone #24

Closed martyychang closed 6 years ago

martyychang commented 6 years ago

I want to be confident in the first release that users' time zones will be respected for all commands, especially since the default time zone is "America/New_York". To make sure time zone-specific results are returned, Apex tests should be written to confirm expected behavior for a Slack user who is in the "America/Los_Angeles" time zone.

martyychang commented 6 years ago

This is confirmed to be a problem that needs to be resolved. I tried to write tests for #18 using a user for whom the Slack API would return the "America/Chicago" time zone, and the test failed when comparing expected Datetime values.

System.AssertException: Assertion Failed: Start Time: Expected: 2018-01-14 13:30:00, Actual: 2018-01-14 14:30:00

martyychang commented 6 years ago

A full suite of tests would be great if it can cover the following scenarios

martyychang commented 6 years ago

Trying to run one test by clocking in as a brand new user, but I'm getting a familiar nuisance error...

System.CalloutException: You have uncommitted work pending. Please commit or rollback before calling out

martyychang commented 6 years ago

I tried wrapping the "when" code in my test inside a runAs block as follows, but I'm still getting the same error.

        System.runAs(apiGuestUser) {
            result = SlashclockService.getInstance(inbound).execute(inbound);
        }
martyychang commented 6 years ago

i confirmed what's happening is that when processing a command for a brand new user, the SlackService.findOrCreateAccount method is creating a new account and then calling the Slack API to determine the user's time zone. Given Salesforce's recommendation on how to resolve the issue, I feel if I solve #37 first by asynchronously fetching users' time zones, I can completely avoid this error altogether.