Skip to content

Conversation

@jtojnar
Copy link
Collaborator

@jtojnar jtojnar commented Jun 7, 2025

We introduced these tests as plan for fixing #56 and #83 but have not found time to fix them yet. Let’s ignore the known bad tests for now.

jtojnar added 2 commits June 7, 2025 20:45
This was introduced in a2ccdcf (#83) but has not been fixed so far.

When a group is created inside the modifier, clicking the create button will cause one more fieldset to be created than there are containers:

```html
  <form action="/" method="post" id="frm-form">

  <fieldset>
  <legend>Team member #1</legend>

  <table>
  <tr>
  	<th><label for="frm-form-members-0-name">Name</label></th>

  	<td><input type="text" name="members[0][name]" id="frm-form-members-0-name" class="text"></td>
  </tr>
  </table>
  </fieldset>

  <fieldset>
  <legend>Team member #2</legend>

  <table>
  <tr>
  	<th><label for="frm-form-members-1-name">Name</label></th>

  	<td><input type="text" name="members[1][name]" id="frm-form-members-1-name" class="text"></td>
  </tr>
  </table>
  </fieldset>

  <fieldset>
  <legend>Team member #3</legend>
  </fieldset>

  <table>
  <tr>
  	<th></th>

  	<td><input type="submit" name="members[multiplier_creator]" class=" button" value="add" formnovalidate data-nette-validation-scope='["members"]'></td>
  </tr>
  </table>

  <input type="hidden" name="_do" value="form-submit">
  </form>
```

```
 Test  tests/unit/CreateButtonTest.php:testFieldsets
After adding a container, there should be two fieldsets.
Failed asserting that actual size 3 matches expected size 2.
```
The nested multipliers were fixed in 39725d3 but the change broke other features so it was reverted in 71cc347 (keeping the test change since it was what we want). See #92 for some more details.

Another regression test was introduced in 1e6e3b2 to ensure the nested multiplier support is correctly implemented.

However, proper implementation has not manifested so far so let’s mark the tests as incomplete until then.
@jtojnar jtojnar force-pushed the incomplete-tests branch from 55f9843 to 853e7c8 Compare June 7, 2025 19:08
@jtojnar jtojnar merged commit 853e7c8 into master Jun 7, 2025
15 of 17 checks passed
@jtojnar jtojnar deleted the incomplete-tests branch June 7, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants