Convert web tests to browser tests for datetime and datetime_range modules

May be premature since we have some major work going on with this, but want to start to identify what this needs for conversion. Being able to mock some of the classes for consistent tests will be a huge benefit for flushing out any remaining timezone problems.

Here is just the base class change. It is going to fail b/c the xpath and $this::url.

Note to anyone else working on this, the `renames = copies` bit in https://www.drupal.org/node/1542048 is important to make sure we have minimal (ha!) rebase / merge problems once other patches start landing.

Doing some housekeeping, and postponing this on #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests. Also note that they are currently in the exclusion list in #2770921: Feb 21st: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits.

When that lands, I think we need to make the decision on how to proceed with this. I think that is the only main blocking issue (also need to replace $this->url in a few places with the one from the session).

DateTimeFieldTest and DateRangeFieldTest are monsters. I think at the very least, we need to refactor them to split out the item/field, widget, and formatter tests into separate classes, and make the decision as to whether to use UTB, KTB, or BTB. And at the same time, clean up some of the kernel-style things that are being done in UI tests (note how in #2811725: Error when render Datetime Range field: Error: Unsupported operand types how this caused something to slip through the cracks).

Here is where we stand with rename + #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests + `$this->url => $this->getSession()->getCurrentUrl()`.

Not sure why we aren't getting past the $this->drupalLogin() here.

So, this is essentially blocked on drupalPostAjaxForm() support. We also need to figure out what to do with DateTestBase::renderTestEntity(), in particular the setRawContent() call.

Keep forgetting to tag issues.

#13 see https://www.drupal.org/node/2809181#comment-11954258 for an idea I had about how to rework $this->renderTestEntity().

Your idea looks better, though, at quick glance.

Green patch! Sa-weet!

Partial review, a lot of the same comment apply to the DateRange version.

1. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -115,8 +115,8 @@ public function testDateField() {
   -              $this->renderTestEntity($id);
   -              $this->assertFieldByXPath('//time[@datetime="' . $expected_iso . '"]', $expected, SafeMarkup::format('Formatted date field using %value format displayed as %expected with %expected_iso attribute.', ['%value' => $new_value, '%expected' => $expected, '%expected_iso' => $expected_iso]));
   +              $output = $this->renderTestEntity($id);
   +              $this->assertContains($expected_iso, $output, SafeMarkup::format('Formatted date field using %value format displayed as %expected with %expected_iso attribute.', ['%value' => $new_value, '%expected' => $expected, '%expected_iso' => $expected_iso]));
   

   I'm worried we are losing coverage here. The XPath assertion made sure the time element was actually rendered out. This justs checked that the ISO string is there.

2. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -215,7 +215,7 @@ public function testDatetimeField() {
   -    $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend//text()', $field_name, 'Fieldset and label found');
   +    $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found');
   

   Is this needed?

3. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -259,8 +259,8 @@ public function testDatetimeField() {
   -            $this->renderTestEntity($id);
   -            $this->assertFieldByXPath('//time[@datetime="' . $expected_iso . '"]', $expected, SafeMarkup::format('Formatted date field using %value format displayed as %expected with %expected_iso attribute.', ['%value' => $new_value, '%expected' => $expected, '%expected_iso' => $expected_iso]));
   +            $output = $this->renderTestEntity($id);
   +            $this->assertContains($expected_iso, $output, SafeMarkup::format('Formatted date field using %value format displayed as %expected with %expected_iso attribute.', ['%value' => $new_value, '%expected' => $expected, '%expected_iso' => $expected_iso]));
   

   Ditto about time.

4. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -368,7 +368,7 @@ public function testDatelistWidget() {
   -    $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend//text()', $field_name, 'Fieldset and label found');
   +    $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found');
        $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found');
   

   Ditto about needed.

5. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -381,7 +381,7 @@ public function testDatelistWidget() {
   -    $this->drupalPostAjaxForm(NULL, [], $field_name . "_settings_edit");
   +    $this->drupalPostForm(NULL, [], $field_name . "_settings_edit");
   

   Hmmm. I think I am OK with this change. Not really sure why this needed Ajax in the first place.

6. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -407,7 +407,7 @@ public function testDatelistWidget() {
   -    $this->drupalPostAjaxForm(NULL, [], $field_name . "_settings_edit");
   +    $this->drupalPostForm(NULL, [], $field_name . "_settings_edit");
        $this->assertFieldByXPath($xpathIncr, NULL, 'Increment element found for Date and time.');
   

   Ditto about Ajax.

7. +++ b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php
   @@ -577,16 +577,16 @@ public function testAlldayRangeField() {
   -  public function testDatelistWidget() {
   +  public function xtestDatelistWidget() {
   

   Comment out test :) Happens in a bunch of places in the DateRange test.

Overall, this looks great. Mainly worried about losing coverage on the time element.

I am 98% sure that this is good, but I want to give this a close study to make sure we aren't regressing anything with xpath test changes.

We also need to create a f/up plan to figure out how to make these more managble. 1000+ line test classes are not good.

OK, for my own sanity, I verified that the tests are actually being run, https://dispatcher.drupalci.org/job/drupal_patches/8878/consoleFull

1. +++ b/core/modules/datetime/tests/src/Functional/DateTestBase.php
   @@ -153,6 +153,9 @@ protected function createField() {
   +   *
   +   * @return string
   +   *   The rendered HTML output.
   

   Because of needing to rework what renderTestEntity does in order for a minimal conversion, this is an in-scope-change.

2. +++ b/core/modules/datetime/tests/src/Functional/DateTestBase.php
   @@ -161,9 +164,7 @@ protected function renderTestEntity($id, $view_mode = 'full', $reset = TRUE) {
   -    $output = $this->container->get('renderer')->renderRoot($build);
   -    $this->setRawContent($output);
   -    $this->verbose($output);
   +    return (string) $this->container->get('renderer')->renderRoot($build);
   

   This could potentially be done with some Mink shenanigans to do the exact same thing, but I this this is a necessary change. The link.module conversion is also doing the same thing.

3. +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
   @@ -115,8 +115,9 @@ public function testDateField() {
   -              $this->renderTestEntity($id);
   -              $this->assertFieldByXPath('//time[@datetime="' . $expected_iso . '"]', $expected, SafeMarkup::format('Formatted date field using %value format displayed as %expected with %expected_iso attribute.', ['%value' => $new_value, '%expected' => $expected, '%expected_iso' => $expected_iso]));
   +              $output = $this->renderTestEntity($id);
   +              $expected_markup = '<time datetime="' . $expected_iso . '" class="datetime">' . $expected . '</time>';
   +              $this->assertContains($expected_markup, $output, SafeMarkup::format('Formatted date field using %value format displayed as %expected with %expected_iso attribute.', ['%value' => $new_value, '%expected' => $expected, '%expected_iso' => $expected_iso]));
                  break;
   

   Not totally happy with this, but scoping this as a conversion with minimal refactoring, I think this is a necessary change. It make this a little more brittle, but I don't see a better alternative.

4. +++ b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php
   @@ -1,12 +1,12 @@
    use Drupal\Component\Render\FormattableMarkup;
    use Drupal\Component\Utility\Unicode;
    use Drupal\Core\Datetime\DrupalDateTime;
    use Drupal\Core\Datetime\Entity\DateFormat;
   -use Drupal\datetime\Tests\DateTestBase;
   +use Drupal\Tests\datetime\Functional\DateTestBase;
    use Drupal\datetime_range\Plugin\Field\FieldType\DateRangeItem;
    use Drupal\entity_test\Entity\EntityTest;
    use Drupal\field\Entity\FieldConfig;
   

   Nit that can be fixed on commit, should be alphabetical.

5. +++ b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php
   @@ -434,7 +440,7 @@ public function testAlldayRangeField() {
   -    $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend//text()', $field_name, 'Fieldset and label found');
   +    $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found');
   

   See #16.

I went through this pretty carefully, and think we have as close to a one-to-one conversion as we are going to get. These tests work, but are a big mess. Adding Needs Followup so we can plan out how to refactor these now that the BTB conversion is done. Will make that issue later.

This is RTBC from my perspective.

Very, very good point.

Retriggered b/c the JSTB intermittents, https://www.drupal.org/pift-ci-job/637557

Here we go.

$ git checkout -b 2780063-37 8.3.x
$ git apply --index -3 2780063-32.patch
error: patch failed: core/modules/datetime/src/Tests/DateTimeFieldTest.php:1
Falling back to three-way merge...
Applied patch to 'core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php' with conflicts.
error: patch failed: core/modules/datetime_range/src/Tests/DateRangeFieldTest.php:165
Falling back to three-way merge...
Applied patch to 'core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php' with conflicts.
U core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php
U core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php

The conflict is from a test change in #2775669: Clean up redundant methods in datetime field formatters b/c changing the daterange formatters from #plain_text to #markup. DateRangeFieldTest is going to fail here, but this is to show clean change.

And here is that hunk removed.

#40, up in #24 @klausi said

I think the conversions for drupalPostAjaxForm() are fine because we did not test javascript functionality anyway. We are just testing the HTML of the response, which is the same when posting the form normally and via AJAX.

I agree with this; and this is just the backport patch now. The followup that I need to make is to break up and refactor DateTimeFieldTest and DateRangeFieldTest into more manageable chunks, and see if we can refactor some of the common bits (one or two other issues will make this easier), and also split out some of these into KTB tests. When we do that, we can add some JSTB where needed.