In tests, move forms inline to cut down on verbosity

[camertron]

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

In the tests for the forms framework, it's common to see this pattern:

class Primer::Forms::SomeTest < Minitest::Test
  class SomeForm < ApplicationForm
    form do |some_form|
      some_form.field(...)
    end
  end

  def test_hidden_checkbox_group
    render_in_view_context do
      primer_form_with(url: "/foo") do |f|
        render(SomeForm.new(f))
      end
    end

    assert_selector ".some-selector"
  end
end

The framework makes us define a separate class for our form. There are a couple of drawbacks to this, at least in tests:

1. The form and the test are defined in separate places, making it difficult to "see" all the logic at play.
2. Forms are designed to be re-usable, hence why they are defined as classes. In tests however, forms are usually specific to a single test and are not re-used in other tests.

This PR adds the ability to define forms inline:

class Primer::Forms::SomeTest < Minitest::Test
  def test_hidden_checkbox_group
    render_in_view_context do
      primer_form_with(url: "/foo") do |f|
        render_inline_form(f) do |some_form|
          some_form.field(...)
        end
      end
    end

    assert_selector ".some-selector"
  end
end

Now, the form is defined inside the test, making it easier to reason about the test as a whole. There are probably too many nesting levels at play here, but that's a problem for another day.

There are a couple of obvious drawbacks with this approach:

1. You cannot pass arguments to the form's constructor on initialization, meaning there's no way to encapsulate data in an inline form instance.
2. Inline forms cannot define any methods, including instance or class methods.
3. There is no way to retain a reference to the class that is dynamically defined for the inline form, meaning it has to be defined anew every time the template is rendered. That's not an issue in tests, but certainly an issue for a production app. So yeah, don't use this in prod.

If you need to circumvent any of these drawbacks, define a form class.

Integration

No changes necessary in production. This change is largely test-only.

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

Accessibility

• No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

• Added/updated tests
  - [ ] Added/updated documentation
  - [ ] Added/updated previews (Lookbook)
  - [ ] Tested in Chrome
  - [ ] Tested in Firefox
  - [ ] Tested in Safari
  - [ ] Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: d45fd7e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR