This issue is part of #2454513: [meta] Make Drupal 8 work with SQLite.

We need to fix this test on SQLite:

Drupal\views\Tests\Handler\ArgumentDateTest

It fails because SQLite does not have a native ISO week substitution string so we have to bend over backwards to generate it.

CommentFileSizeAuthor
#7 interdiff.txt2.94 KBamateescu
#7 2462481-7.patch3.38 KBamateescu
#1 2462481.patch1.13 KBamateescu

Comments

amateescu’s picture

amateescu commented
Status: Active » Needs review
StatusFileSize
new 2462481.patch1.13 KB

This should do it. Thanks to @daffie for pointing me in the right direction in #2454753-4: [Meta] SQLite: Fix sub-system tests in the views module.

Test summary
------------

Drupal\views\Tests\Handler\ArgumentDateTest         50 passes       40 messages

Test run duration: 11 sec

Status: Needs review » Needs work

The last submitted patch, 1: 2462481.patch, failed testing.

Status: Needs work » Needs review

amateescu queued 1: 2462481.patch for re-testing.

amateescu’s picture

amateescu commented

Unrelated test fail: File /tmp/some-temp-file.jpg could not be copied to temporary://some-temp-file.jpg.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented 
  1. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -1797,7 +1797,15 @@ public function getDateFormat($field, $format) {
+        // @see http://stackoverflow.com/a/15511864/1499564

    It may be better to link to http://en.wikipedia.org/wiki/ISO_week_date#Calculation 

  2. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -1797,7 +1797,15 @@ public function getDateFormat($field, $format) {
+          $expression = "((strftime('%j', date(strftime('%Y-%m-%d', $field, 'unixepoch'), '-3 days', 'weekday 4')) - 1) / 7 + 1)";

    Are there any additional unit tests that could be added to test some odd years so that we know that this calculation is accurate based on the calculation linked above?

daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

amateescu’s picture

amateescu commented
StatusFileSize
new 2462481-7.patch3.38 KB
new interdiff.txt2.94 KB

@mradcliffe, thanks for the pointer, added that link and a couple more tests.

Test summary
------------

Drupal\views\Tests\Handler\ArgumentDateTest                   52 passes                           44 messages

Test run duration: 12 sec
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Nice work @amateescu. This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 4c9e0f9 and pushed to 8.0.x. Thanks!

  • alexpott committed 4c9e0f9 on 8.0.x 
    Issue #2462481 by amateescu: SQLite: Fix views\Tests\Handler\...
mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

I was going to run ArgumentDateTest on PostgreSQL as well, but then I couldn't figure out why the patch wasn't applying until I realize it had already been committed. Hah.

ArgumentDateTest
52 passes, 0 fails, 0 exceptions, 44 debug messages

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.