mbraak/django-file-form

Tests break due to #444 in version 3.2.1

BoPeng opened this issue · 9 comments

After updating to version 3.2.1, my tests fail due to the following new function,

def clean(self, data, initial=None):
return super().clean(data + (initial or []))

which triggers error

       TypeError: unsupported operand type(s) for +: 'NoneType' and 'list'

if the field is passed with data None (no file specified).

I am not sure what is the motivation for this piece of code since the super() class FileField accepts

super().clean(data, initial)

I'll have a look.

Can you tell me more about the test? Is it a selenium test or a unit test?

In my case I have a form

class MyForm(FileFormMixin):
    main_notebook = UploadedFileField(required=False, accept='.ipynb')
    attachments = MultipleUploadedFileField(required=False)

and I have been testing the fields with

    form = MyForm(data=dict(
        main_notebook=None,
        attachments=[],
    )
    assert form.is_valid()

among other fields. The test stopped working with the new MultipleUploadedFileField.clean() function. I tried both None and [] for both fields.

On a related note, I am not quite sure how to pass any real file objects (UploadedFile or PlaceholderUploadedFile) to the form to avoid having to test the form with selenium tests. I would appreciate any suggestion if you have done so.

Thanks. I added a pr to fix the test: #456

I could not reproduce the original error. The pr contains a test, but it succeeds with and without the change.

One thing I noticed about the test code above. The class MyForm doesn't descend from Form. I would expect: class MyForm(FileFormMixin, Form)

I will look for a way to test the form with UploadedFile and PlaceholderUploadedFile

Thanks. My real form and test is a lot more complicated so something else might be going on. I did trace to the clean function but did not try to create a test case. Anyway, I can confirm that

  1. My tests pass with django-file-form 3.2.0
  2. break with django-file-format 3.2.1, and
  3. and pass again with the PR.

Did some more experiments, I can confirm that the problem persisted (only with django-file-form=3.2.1) after stripping my form down to

class MyForm(FileFormMixin, forms.Form):
    main_notebook = UploadedFileField(required=False)
    attachments = MultipleUploadedFileField(required=False)

and test down to

def test_my_form():
    form = forms.MyForm(data=dict(
        main_notebook=None,
        attachments=[],
    ))
    assert form.is_valid()

for django==2.2.19, but ok for django==3.2.1, so the PR would be needed for django2.

Ok, thanks. I will merge the pr.

I added an example test with an uploaded file in test_form.py.

    def test_upload_notebook(self):
        uploaded_tus_file = UploadedTusFile(
            file=ContentFile("xyz", "test.txt"), file_id="111"
        )
        files = dict(main_notebook=uploaded_tus_file)

        form = TestForm(data=dict(), files=files)

        self.assertTrue(form.is_valid())
        self.assertEqual(form.cleaned_data["main_notebook"].name, "test.txt")

The pr is merged