[META] Deprecate assertWaitOnAjaxRequest() and make the JsWebAssert::waitFor*() methods behave like real assertions

I pointed @phenaproxima to #1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS. If we can confirm that this is is caused by that, perhaps we can finally get that issue fixed :) OTOH it's already marked Major and I don't think it can possibly meet the requirements for Critical.

Nice. Could get a waitForTextRemoved() method too?

@bnjmn is using one for #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest

He calls it WaitForNoText(), but I think to be more consistent waitForTextRemoved() is better.

Or should that be a different issue?

Perhaps it's an antipattern, but I have used $this->assertEmpty($assert_session-> waitForElementVisible('css', '.something', 1000)); The change to waitForElementVisible() will break those types of assertions. What should they be replaced with?

I guess waitForElementRemoved(), which is being added in 8.8?

I think waitForElementRemoved():
https://www.drupal.org/node/2906685

Removing most (if not all) of the $this->assertNotEmpty()and $this->assertNotNull() code wrapped around JsWebAssert::waitFor*() methods.

Broke off a side issue for one of the failures: #3102981: Invalid selector in EditModeTest

Fixing a test or two, fixing some missed assertions testing the result of the updated methods which need to be removed or replaced.

Adding another side issue: #3102990: FormGroupingElementsTest::testVerticalTabChildVisibility passing erroneously

Postponing on

• #3102990: FormGroupingElementsTest::testVerticalTabChildVisibility passing erroneously
• #3102981: Invalid selector in EditModeTest

We can't really fix this one until those are fixed. But we should keep going through the failures here to see if there are similar issues where tests were passing erroneously.

• Moving methods to JsWebAssert that were marked as @todos for this issue
• Fixing some more tests.

more fixes

making method public

restoring one of the methods removed from the MediaLibraryTestBase.

making a method public, same mistake as #28 on a different method.

Breaking off one part of this as a separate issue: #3103034: Invalid selector in MaximumFileSizeExceededUploadTest

I didn't include it in the file name, because I just created it.

Also, I checked and there are 240 occurrences of assertWaitOnAjaxRequest() in core. Given that this issue is already large, I think we should come up with a way of splitting this up.

Here's how we might do that:

• Revert the changes where we're moving classes from media library and move that into a separate issue. (See #43 where I did this.)
• Add a separate issue to deprecated assertWaitOnAjaxRequest, so that could go into a separate issue and actually remove the usages of assertWaitOnAjaxRequest

Another thing we could do is make this a META issue.

Fixing the failure, and marking as postponed.

I think (IMHO), this should be a META issue, and we can do it as a series of child issues.

Deprecating assertWaitOnAjaxRequest().

Update: I guess this isn't enough, we'll need to add a @trigger_error: https://www.drupal.org/core/deprecation.

Adding @trigger_error to assertWaitOnAjaxRequest().

1) In #25 I moved some methods from MediaLibraryTestBase to JsWebAssert. I think some of that was out of scope, so reverting those changes. I opened two separate issue to explore moving two of these methods to JsWebAssert:

• #3103097: Move waitForNoText from MediaLibraryTestBase to JSWebAssert
• #3103096: Move waitForElementsCount from MediaLibraryTestBase to JSWebAssert

2) The methods in JsWebAssert to which we added assertions, if they were returning a boolean, no longer need a return. If they fail, they'll throw an error, so there's no point of a return value, AFAIK. They either pass or throw an error. Those that return a node element can still return it, in case it's needed, but the documentation should be updated to indicate it won't return NULL, but with throw an error, for example:

-   * @return \Behat\Mink\Element\NodeElement|null
-   *   The page element node if found and visible, NULL if not.
+   * @return \Behat\Mink\Element\NodeElement
+   *   The page element node if found and visible, otherwise an error is thrown.

Updating the @todo items based on #43 to refer to those two new issues.

Created a subtask for the work related to updating the assertions: #3103101: Make the JsWebAssert::waitFor*() methods behave like real assertions

This leaves this issue as a META issue. It also allows 3103101 to be committed independent of #3103081: Deprecate JsWebAssert::assertWaitOnAjaxRequest() in Drupal 9.0 and Drupal 8.9 and Replace usages of JsWebAssert::assertWaitOnAjaxRequest() in core, which will take a lot more work to be ready to commit.

Opened #3230726: JsWebAssert::assertWaitOnAjaxRequest() should use Behat\Mink\Exception\ExpectationException not RuntimeException as a short-term solution for problems with assertWaitOnAjaxRequest

Because of the new add_js command, ajax.ajaxing will now wait for the commands to finish executing before being set to false. That should help a bit.