Test base

[mdewhirst]

This test-base branch has a prototype partial test suite covering base testing infrastructure with helpers plus only one method 'totp' set of tests.

The idea is to decide if this is an appropriate way to unit-test django-mfa2. In theory, this will delete tests.py and create a mfa/tests directory. The PR includes a README.md in that tests directory.

[mdewhirst]

@mkialoby, just tried running the tests again after applying the latest mfa2 changes and encountered a csp dependency and a number of errors so please don't try and merge this PR. I'll investigate and get back to you.

[mkalioby]

Thanks for the headsup. I was planning to check it soon.

[mdewhirst]

@mkialoby, I avoided the example app testing errors as follows ...

1. Installed django-csp==3.8 and some errors went away.

However, django-csp has moved on and is now at version 4.0 which is backwardly incompatible with django-csp 3.8. That introduced more errors which were resolved when I migrated from 3.8 to 4.0. Removing django-csp completely resolved all those errors.

2. In the example app, adjusted urlpatterns to include mfa.urls with "mfa/" instead of "mfa2/"

urlpatterns = [
    path("admin/", admin.site.urls),
    #path("mfa2/", include("mfa.urls")),
    path("mfa/", include("mfa.urls")),
    path("auth/login", auth.loginView, name="login"),
    path("auth/logout", auth.logoutView, name="logout"),
    path("devices/add/", TrustedDevice.add, name="add_trusted_device"),
    re_path("^$", views.home, name="home"),
    path("registered/", views.registered, name="registered"),
]

Alternatively, and just as easily, I could adjust 9 tests to change '/mfa/...' to '/mfa2/...'

The way to go depends on your selection of the standard mfa namespace.

If mfa2, I'll adjust the tests.

[mkalioby]

mfa is better options.

[mkalioby]

This is really a weird way to do unit test, as the function in the view shall do email adding, so the code branch inside the package gets executed. Here the test case tests itself and not the written code inside the package. Do you get me concern?

[mdewhirst]

I'll have close look tomorrow, but in general I've refactored all common stuff out of the tests into base class methods so each test only needs to do what it advertises. Also, there's a lot of extra tests written now and I'll update the PR shortly. I think I only have a dozen or so to write and we'll have most of mfa covered. It is going to be a massive task to settle it down before it can be merged. There's more than 200 tests at this stage. Sent from Proton Mail Android

…

-------- Original Message --------

On 10/9/25 18:55, Mohamed El-Kalioby wrote: @mkalioby commented on this pull request. --------------------------------------------------------------- In [mfa/tests/test_view_email.py](#99 (comment)): > + session.save() + + # Verify no email key exists + self.assertFalse( + User_Keys.objects.filter(username=self.username, key_type="Email").exists() + ) + + # Test the core logic by simulating successful authentication + # Simulate the POST request logic from Email.auth() + username = session["base_username"] + token = "123456" + + # Simulate the validation logic + if session["email_secret"] == token.strip(): + # Simulate key creation when MFA_ENFORCE_EMAIL_TOKEN is True + uk = User_Keys() This is really a weird way to do unit test, as the function in the view shall do email adding, so the code branch inside the package gets executed. Here the test case tests itself and not the written code inside the package. Do you get me concern? — Reply to this email directly, [view it on GitHub](#99 (review)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AANG4B6EYOFOOHPC2JQO7QD3R7RP5AVCNFSM6AAAAAB7SDLPCOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTEMBVGE2TGNZSG4). You are receiving this because you authored the thread.Message ID: ***@***.***>

[mkalioby]

Sure Mike, tyt.

Thanks for the great work.

I tried coverage to see the missed lines.

You can try that to check what I'm talking about.

[mdewhirst]

@mkialoby, I avoided the example app testing errors as follows ...

1. Installed django-csp==3.8 and some errors went away.

However, django-csp has moved on and is now at version 4.0 which is backwardly incompatible with django-csp 3.8. That introduced more errors which were resolved when I migrated from 3.8 to 4.0. Removing django-csp completely resolved all those errors.

2. In the example app, adjusted urlpatterns to include mfa.urls with "mfa/" instead of "mfa2/"

urlpatterns = [
    path("admin/", admin.site.urls),
    #path("mfa2/", include("mfa.urls")),
    path("mfa/", include("mfa.urls")),
    path("auth/login", auth.loginView, name="login"),
    path("auth/logout", auth.logoutView, name="logout"),
    path("devices/add/", TrustedDevice.add, name="add_trusted_device"),
    re_path("^$", views.home, name="home"),
    path("registered/", views.registered, name="registered"),
]

Alternatively, and just as easily, I could adjust 9 tests to change '/mfa/...' to '/mfa2/...'

The way to go depends on your selection of the standard mfa namespace.

If mfa2, I'll adjust the tests.

[mdewhirst]

Getting ready to deliver a smaller PR so it can be reviewed more easily.