Page MenuHomeSoftware Heritage

Remove use of app.test_client() as a ctx-manager
ClosedPublic

Authored by vlorentz on Thu, Aug 4, 10:11 AM.

Details

Summary

According to
https://github.com/pallets/flask/issues/4734#issuecomment-1204124984,
app.test_client() should not be used as a long-lived context manager.

This causes swh-indexer's tests to break with Flask >= 2.2.0 because
some of them start threads within the lifespan of the context manager,
which causes a crash.

As far as I can tell, simply assigning app.test_client() instead of
entering does not break anything.

Resolves T4416.

Diff Detail

Repository
rDCORE Foundations and core functionalities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D8177 (id=29527)

Rebasing onto b805b3106f...

Current branch diff-target is up to date.
Changes applied before test
commit c824e4b66fc540c64af886809a1e7821890fdde0
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Aug 4 10:09:35 2022 +0200

    Remove use of app.test_client() as a ctx-manager
    
    According to
    <https://github.com/pallets/flask/issues/4734#issuecomment-1204124984>,
    app.test_client() should not be used as a long-lived context manager.
    
    This causes swh-indexer's tests to break with Flask >= 2.2.0 because
    some of them start threads within the lifespan of the context manager,
    which causes a crash.
    
    As far as I can tell, simply assigning app.test_client() instead of
    entering does not break anything.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/459/ for more details.

ardumont added a subscriber: ardumont.

Let's try and see if that fixes it then.

This revision is now accepted and ready to land.Thu, Aug 4, 10:18 AM