Contrib.social

PHPUnit tests: Support form actions being outside of a form

Open on Drupal.org →
Created on 21 June 2024, 3 months ago
Updated 13 August 2024, 25 days ago

Problem/Motivation

Gin has a new feature to display all form items in a sticky bar which resides outside of the actual <form>. This is natively support by HTML but will fail regular functional tests using submitForm() as this assumes the form actions being part of the <form>.

We temporarily override the submitForm() so the PHPUnit tests within the Gin theme do not fail. However if you run any functional testing on the backend it will now potentially fail.

I can think of a couple of ways how we can easily solve this.

Steps to reproduce

Approach A:
Install Gin theme
Enable sticky form actions
Run PHPUnit tests on an admin form

Approach B:
Create form where button render outside the form actions
like:

<form action="/action_page.php" method="get" id="form1">
  What is your favorite color? <input type="text" name="fav_color"><br>
  <input type="submit">
</form>

<fieldset form="form1">
  Name: <input type="text" name="username"><br>
  Email: <input type="text" name="usermail"><br>
</fieldset>

Reference link: https://www.w3schools.com/tags/att_form.asp

Now if you try to add Phpunit test case for this form , then test fails.

Proposed resolution

Alter submitForm() to support form actions being set outside of a <form>.

Remaining tasks

Update submitForm()

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Needs work

Version

11.0 🔥

Component
Form  →

Last updated 1 minute ago

Created by

🇨🇭Switzerland saschaeggi Zurich

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @saschaeggi
  • Merge request !8482Resolve #3456202 "Phpunit tests support" → (Open) created by saschaeggi
  • Comment 3 months ago →
    🇨🇭Switzerland saschaeggi Zurich
  • Pipeline finished with Failed
    3 months ago
    Total: 518s
    #204663
  • Pipeline finished with Failed
    3 months ago
    Total: 171s
    #204684
  • Pipeline finished with Success
    3 months ago
    Total: 576s
    #204693
  • Comment 3 months ago →
    🇨🇭Switzerland saschaeggi Zurich
  • Comment 2 months ago →
    🇨🇭Switzerland saschaeggi Zurich
  • Status changed to Needs review 2 months ago
  • Comment 2 months ago →
    🇫🇷France andypost
  • Status changed to RTBC 2 months ago
  • Comment 2 months ago →
    🇫🇷France Grimreaper France 🇫🇷

    Hi,

    I was blocked when trying to implement tests on a custom project which is using Gin.

    I copied the submitForm method from the MR in the tests of my project and now it is working.

    Would it be possible to make it a trait please?

    Thanks!

  • Comment 2 months ago →
    🇫🇷France Grimreaper France 🇫🇷

    Uploading patch for composer usage.

  • Status changed to Needs review 2 months ago
  • Comment 2 months ago →
    🇬🇧United Kingdom catch

    This looks sensible and doesn't break any existing tests. However is there a minimal implementation of the form button being outside the form we could implement in a test module so that we could add coverage for it to BrowserTestBaseTest?

  • Comment 2 months ago →
    🇺🇸United States nicxvan
  • Status changed to Needs work 2 months ago
  • Comment 2 months ago →
    🇨🇦Canada b_sharpe
  • Pipeline finished with Success
    2 months ago
    Total: 553s
    #215223
  • Status changed to Needs review 2 months ago
  • Comment 2 months ago →
    🇨🇦Canada b_sharpe

    Added tests to go along, since all changes are in tests, the Gitlab "Test Only" runner still passed haha, so just for the sake of it, here's a "Test only" patch omitting the UI helper changes to show the failure.

  • Comment 2 months ago →
    pooja_sharma
  • Comment 2 months ago →
    pooja_sharma

    I have tested the issue & it's replicable on local

    Test both test scenario pass & fail & attached the screenshots for reference:

    A. Keep the newly added test code on local , but not fixation code: issue replicable (test fail)

    B. Keep the newly added test code on local , also added fixation code: issue fixed (test pass)

    RTBC++1, also reviewed test code , written with minimal required changes

  • Comment 2 months ago →
    🇺🇸United States nicxvan

    @ pooja_sharma

    Can you clarify what you are splitting into the fix code vs test code?

  • Comment 2 months ago →
    🇨🇦Canada b_sharpe

    @nicxvan it's explained in #12, because the code is all part of tests, the test-only gitlab test will always pass, so @pooja_sharma used my test-only patch which is the actual new test to show fail, and then combined the entire MR (the bugfix) to show pass

  • Comment 2 months ago →
    pooja_sharma

    Yeah, Thanks @b_sharpe you got it right. , however for the clarity, @nicxvan I have updated comment.

  • Comment 2 months ago →
    🇺🇸United States nicxvan

    Ah I missed the test only patch, that's why I was confused. Thanks for clarifying.

  • Comment 2 months ago →
    🇦🇺Australia dpi Perth, Australia

    I havent reviewed the test, but can say the associated changes resolve tests where the Gin interface is involved. Particularly form submissions, resulting in the failure:

    Behat\Mink\Exception\ElementNotFoundException: Element matching xpath "./ancestor::form" not found.

    Thanks!

  • Comment about 2 months ago →
    🇳🇿New Zealand quietone New Zealand

    Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies. Also, 10.2 is in security mode now.

  • Status changed to Needs work about 2 months ago
  • Comment about 2 months ago →
    needs-review-queue-bot

    The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Pipeline finished with Success
    about 2 months ago
    Total: 529s
    #227895
  • Merge request !8838Resolve #3456202 "Test phpunits ouside form action btn" → (Open) created by pooja_sharma
  • Pipeline finished with Success
    about 2 months ago
    #228647
  • Status changed to Needs review about 2 months ago
  • Comment about 2 months ago →
    pooja_sharma

    As prev MR has some unrelated 1000+ additional change, it seems MR got corrupted.

    Raised new MR 8838 with related changes only, against 11.x dev target branch

    For testing I have already added detail comments along with attached the screenshots in #14 ✨ PHPUnit tests: Support form actions being outside of a form Active

  • Status changed to Needs work 25 days ago
  • Comment 25 days ago →
    🇺🇸United States smustgrave

    Not sure which MR is up for review.

    Can one be hidden please and whichever one is it be pointed to 11.x?

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024