StatisticsViewsResult should only deal in integer values

Works for me.

Thanks! It's updating one assertion, but can we also add an explicit test that we always get integer output even when the input is strings?

@xjm means: construct a StatisticsViewsResult object, pass 3 strings, verify you get back 3 integers when you use the 3 getters.

My inclination is that that seems a disproportionate amount of testing for the bugfix. OTOH, this appears to not be a problem in core, only with migrate? Then isn't this a bug in migrate?

Re #7: it isn't so much a bug in migrate as a bug in the statistics API that migrate is using in a test. Back to NW for #4.

This will also need a re-roll if/when #2930101: i18n / statistics - node counter not updated for translations lands. Since it is critical, let's soft-block here. Or get this in super fast. Here's a patch with the added tests.

LGTM

1. +++ b/core/modules/statistics/tests/src/Unit/StatisticsViewsResultTest.php
   @@ -0,0 +1,46 @@
   +  public function testStatisticsCount() {
   +    $this->assertNodeCounter(2, 0, 1421727536);
   +    $this->assertNodeCounter(1, 0, 1471428059);
   +    $this->assertNodeCounter(1, 1, 1478755275);
   +    $this->assertNodeCounter('1', '1', '1478755275');
   +  }
   ...
   +   * @param int $total_count
   +   *   The expected total count.
   +   * @param int $day_count
   +   *   The expected day count.
   +   * @param int $timestamp
   +   *   The expected timestamp.
   

   int|string right? as that's the point of the test to assert that if strings are input then ints are output.

   Also I think this test would be better as a data provider rather than a single test.

2. +++ b/core/modules/statistics/tests/src/Unit/StatisticsViewsResultTest.php
   @@ -0,0 +1,46 @@
   +   * @param int $nid
   +   *   The node ID.
   

   This param is not passed.

+++ b/core/modules/statistics/tests/src/Unit/StatisticsViewsResultTest.php
@@ -0,0 +1,52 @@
+
+

Should be only one empty line

I don't think assertNodeCounter is needed as it's used only once.

removed the extra empty line as per comment #15.
Here is the patch and interdiff.

+++ b/core/modules/statistics/tests/src/Unit/StatisticsViewsResultTest.php
@@ -0,0 +1,51 @@
+  protected function assertNodeCounter($total_count, $day_count, $timestamp) {
...
+    $this->assertSame((int) $total_count, $statistics->getTotalCount());
+    $this->assertSame((int) $day_count, $statistics->getDayCount());
+    $this->assertSame((int) $timestamp, $statistics->getTimestamp());

What about providing also the expected values?

I'm not sure about adding assertNodeCounter as a seperate method, it doesn't add any value to extract this, as it's only used in one place. I already mentioned this in #15. If we do that, I feel like @dawehner's remark will be resolved as well.

Awesome, looks great!

Testbot Snafu.

Testbot issue.

Committed and pushed c01bed2b2d to 8.6.x and c71f50f5ad to 8.5.x. Thanks!