Closed (fixed)
Project:
Search API Autocomplete
Version:
8.x-1.x-dev
Component:
Tests
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
17 Jul 2018 at 08:14 UTC
Updated:
12 Nov 2019 at 16:39 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
idebr commentedThe biggest change when switching to WebDriverTestBase is you can no longer inspect response status codes, see #2857562: Disabled methods to inspect headers for JavascriptTestBase tests
The suggestion in the change record is check for the existence of elements in the UI instead. This is already the case in most of
\Drupal\Tests\search_api_autocomplete\FunctionalJavascript\IntegrationTest, so many of these can be removed without losing too much data.However, a test like \Drupal\Tests\search_api_autocomplete\FunctionalJavascript\IntegrationTest::checkAdminAccess probably better off in a Functional test:
Comment #3
drunken monkeyComment #4
drunken monkeyMaybe like this?
(Didn’t manage to install Chromedriver, so still can’t run JS tests locally – has been the same for years, though, as PhantomJS hasn’t been working for quite some time, either.)
Comment #6
drunken monkeyShoot.
Did anyone manage to get this running and would be able to help here? Would be really appreciated.
Comment #7
idebr commentedAttached patch completes the conversion to WebDriverTestBase
Note that integration with Search API Pages requires the patch from #3063684: Search block form should define a base form ID to be committed.
Comment #8
drunken monkeyExcellent, thanks a lot!
Just seems like something went wrong in patch creation? The attached should work instead.
Comment #10
drunken monkeyShould hopefully work now that #3063684: Search block form should define a base form ID has been committed …
Comment #12
idebr commentedThe Drupal test infrastructure is still downloading the latest tagged release for drupal/search_api_page:
Attached patch updates the package requirement, so it downloads the dev-version instead.
Comment #13
idebr commentedI was using an outdated remote branch for 8.x-1.x. This patch should apply properly.
Comment #14
drunken monkeyThanks for fixing this problem!
Phew, glad I still spotted this by sheer luck! (Not sure how I missed this – I guess I just trusted you and the test bot, since I couldn’t test anyways.)
This is not how
ltrim()works! Let’s usesubstr()instead …Looked over the rest of the code again, but that seems fine. Please test/review the attached revision and we can finally commit.
Comment #15
idebr commentedLooks good!
Comment #17
drunken monkeyThanks for the feedback!
Committed.
Thanks a lot again for all your help here!