Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infer timezone in DateTime field handler #126

Merged
merged 2 commits into from
Mar 20, 2018
Merged

Infer timezone in DateTime field handler #126

merged 2 commits into from
Mar 20, 2018

Conversation

jonathanjfshaw
Copy link
Contributor

I thought I had already made PR for this but I can't find it, forgive me if I'm being foolish.

Currently the following scenario will fail most of the time:

Given I am viewing an article:
  | title                  | test                            |
  | field_datetime | 2005-06-30 14:00:00 |
Then I should see "2005-06-30 14:00:00"

It will typically fail because values are by default created and stored using UTC, but rendered using the current user's timezone. The workaround is for step authors to do mental time math, writing their "Given" times in UTC and their "Then" times in the Drupal timzeone (which is the default for any user).

I think we should assume that step authors intend time values to be in the Drupal default timezone when they write them in Given statements, unless they explicitly specify a timezone.

This will break BC for anyone writing tortured scenarios expecting the current behavior.

I can write some tests for it in the extension, if you like the idea of this.

@jonathanjfshaw
Copy link
Contributor Author

We may want to ensure forwards compatibility with https://www.drupal.org/node/2632040

@jonathanjfshaw
Copy link
Contributor Author

I now know that forward-compatibility with https://www.drupal.org/node/2632040 is not an issue. We could add extra logic for that, but it won't invalidate what I suggest here.

@jhedstrom
Copy link
Owner

Should this go in before or after #167?

@jonathanjfshaw
Copy link
Contributor Author

jonathanjfshaw commented Mar 20, 2018 via email

@jhedstrom jhedstrom added this to the 2.0 milestone Mar 20, 2018
@jhedstrom jhedstrom merged commit e8cc44c into jhedstrom:master Mar 20, 2018
@jhedstrom
Copy link
Owner

Makes sense to me. Thanks for this!

@bkosborne
Copy link

This created an issue in #200

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

Successfully merging this pull request may close these issues.

3 participants