Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surfacing form validation errors in tests #786

Open
abandoned-prototype opened this issue Aug 9, 2020 · 3 comments
Open

Surfacing form validation errors in tests #786

abandoned-prototype opened this issue Aug 9, 2020 · 3 comments

Comments

@abandoned-prototype
Copy link
Collaborator

In a lot of our views that include submitting a form the flow looks something like this

@main.route('/unit/new', methods=['GET', 'POST'])
def add_unit():
    form = AddUnitForm()
    if form.validate_on_submit():
        # [...] create unit
        return redirect(url_for('main.get_started_labeling'))
    else:
        return render_template('add_unit.html', form=form)

The first time accessing the path we show the template with the form, then once it's submitted we do access the same path, but this time do something with the data and redirect to a different url.
The problem is that with form.validate_on_submit() we don't separate the initial/GET case from the validation failed case. So in the case that the validation fails we return the original form, filled with the data and maybe some error message. This is a problem for our integration tests, because it is hard to say what actually went wrong since the error message (if it is shown at all) is hidden somewhere in the generated html code.
For example the test_ac_can_edit_officer_in_their_dept test failed on the line officer = Officer.query.filter_by(last_name=last_name).one() but the reason was that the previous command

rv = client.post(
                url_for('main.add_officer'),
                data=data,
                follow_redirects=True
            )

seemed to have run successfully (status_code 200) when in fact it didn't do the expected work of editing the officer because the validation failed.

I am not sure how to best solve this, but maybe at least writing the validation errors to std-out or std-err might give some visibility while running tests.

@fritzdavenport
Copy link
Contributor

This was referenced Aug 10, 2020
@dismantl
Copy link
Member

It looks to me like a good portion of our tests check for errors by searching for error messages in the output, e.g. assert b'Email already registered' in rv.data, while assuming or asserting a 200 response code. Isn't that standard practice? I would expect form validation errors to still return a 200 response code and include form-wide or field-specific error messages inline.

@parhamr
Copy link
Contributor

parhamr commented Aug 23, 2020

@dismantl in my experience, HTML requests typically return HTTP 400 or 422 with the validation errors, but a JSON API, for example, would always return HTTP 200 but include an errors key in the response object.

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

No branches or pull requests

4 participants