Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Port over drupalPostWithFormat from WTB
Proposed resolution
Remaining tasks
- Review
- Address feedback
- Commit
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#3 | 2795041-3.patch | 3.41 KB | dawehner |
Comments
Comment #2
dawehnerWorking on this particular subissue
Comment #3
dawehnerHere is one implementation using guzzle
Comment #4
dawehnerComment #5
dawehnerComment #6
dawehnerComment #7
klausiWhy do we need this on BrowserTestBase? A test would only use this to test AJAX behavior, which we should convert to JavascriptTestBase instead?
doc block is missing
doc block is missing
let's not use \Drupal in classes when we don't have to.
Does this even use the correct HTTP client of the curent test, like the Simpletest cookie and stuff?
Comment #8
dawehnerThank you for your review!
That's a good question. All calls in core are indeed related with with JS in some form or another:
I just fear for contrib tests they could leverage that also for other
?_format=foo
tests. Do we care about them?Comment #9
dawehnerI'm happy to skip this method, but this might result in more porting work of existing tests.
Comment #10
klausiI think that is fine - testing javascript should really use JavascriptTestBase, so even if porting is harder I think it is totally worth it.
Should we close this in the meantime? We can reopen if we find out that we need this method for something other than javascript testing.
Comment #11
dawehnerOh yeah I totally agree on that, and well, at least for core, this seems to be indeed the case.
I'm just wonder about
drupalGetWithFormat
as that one is certainly used way more often, like in\Drupal\rest\Tests\Views\StyleSerializerTest
which is unrelated to JS.Maybe we should put both together into a trait, and call it a legacy day? To be honest for those tests you want to use guzzle, as you are not using a browser, you care about the HTTP requests.
Comment #12
dawehnerSee #2820182: Add a LegacyBrowserTestBase which uses every legacy trait we have
Comment #13
dawehnerThis is certainly not in review as of now :)
Comment #15
dawehnerNote: We certainly should throw a
trigger_error
here.Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commented#7, #10:
What about security tests? Why should we limit ourselves to the JS-API only? Running security tests in a JS environment can add an additional number of js/browser validators, which may not be on the backend. Accordingly, the tests will give a false sense of security.
I think BTB should support no less diverse API than WTB for the convenience of working with requests in different formats.
The choice of conversion WTB -> BTB or JTB is not related to this issue.
Comment #19
alexpottAdding methods with no usages tends to be problematic also we want to maintain less test API not more.
Comment #20
dawehnerI agree. I think being more explicit than implicit is also often better. One less question of how things work.
Comment #21
alexpottSo since @dawehner and I are both testing maintainers going to close this issue. 1 less in the queue.