flask-executor

flask-executor copied to clipboard

Resolves #50

Gevent-specific tests needs to be run with python -m gevent.monkey --module pytest tests/test_gevent.py

A little explain for 7c42ab7.

Gevent uses a custom _FutureProxy object that implements __getattr__ that ensures some methods like .done(), .cancel(), etc. will be returned by a contained AsyncResult object.

Using object.__gettattributes__ without a fallback fails to retrieve such methods.

FYI if you're seeing the test for "propagate exception callback" failing in your gevent tests:

1. The tests are currently failing because gevent hasn't monkeypatched threading - things like concurrent.futures.wait won't work on gevent Futures without that. Add this at the top of test_gevent.py:

from gevent import monkey
monkey.patch_all()

2. The original test I wrote, even though it appears to pass for the stdlib ThreadPoolExecutor, is fundamentally faulty - see #24. That's not your problem, and fixing the above should be enough for you get your tests passing so this can be merged, but just an FYI.

I originally left the pull request as draft for two reasons:

• I planned to write some example in the documentation but I had not time to check how you are generating it
• There are two ways to make gevent tests pass, one is what you are suggesting, the other is cherry-picking f5fb10f that slightly changes test.yaml allowing them to pass.

~~On the latter, maybe doing just a monkey patch is the simplest way, I'll refactor it to do so.~~ Sorry, the previous sentence was completely incorrect, they have to run separately or gevent.mocking mess up the rest of the tests.

As I wrote above, f5fb10f needs to be merged on master before tests can pass...

      - name: Test with pytest
        run: |
          pytest --cov=flask_executor/ --cov-report=xml --ignore=test_gevent.py
      - name: Gevent-specific test with pytest
        run: |
          python -m gevent.monkey --module pytest tests/test_gevent.py

If you prefer I can just toss gevent tests away from this pull request and open a new one that implements just that.

Since gevent-testing is not strictly needed to implement #50, I'll just leave in this pull request the tests for what the implementation truly does.

I'll try to figure out what's the best way to enable gevent monkeypatch only for some test (if there is a way) and open another PR when it's reasonably done.

I feel the PR is ready to be merged, waiting for your approval @dchevell