Skip to content

test: Fix unexpected failure from bad mocking#6806

Merged
auvipy merged 1 commit intomasterfrom
test/fix-request-stack-mocking
Jun 9, 2021
Merged

test: Fix unexpected failure from bad mocking#6806
auvipy merged 1 commit intomasterfrom
test/fix-request-stack-mocking

Conversation

@maybe-sybr
Copy link
Contributor

@maybe-sybr maybe-sybr commented Jun 8, 2021

Description

This test would attempt to mock the request_stack of a task so as to
confirm that it could confirm that the request object pushed onto it
contained simulated delivery information as expected. However, it did
not wrap the original call target which led to an unfortunate
interaction with the worker optimisations in app/trace.py which would
not find the request on the stack and therefore not end up calling the
task's run() method.

The worker optimisations can be enabled as a side effect of other tests
like test_regression_worker_startup_info() in the mongo and cache
backend suites. This led to a situation where the test changed in the
diff would fail if those tests happened to run before it! Luckily, the
side effect of the worker optimizations being enabled are not what cause
the unrelated failure, the test in this diff was a just a bit unaware of
the consequences of its mocking.

This test would attempt to mock the `request_stack` of a task so as to
confirm that it could confirm that the request object pushed onto it
contained simulated delivery information as expected. However, it did
not wrap the original call target which led to an unfortunate
interaction with the worker optimisations in `app/trace.py` which would
not find the request on the stack and therefore not end up calling the
task's `run()` method.

The worker optimisations can be enabled as a side effect of other tests
like `test_regression_worker_startup_info()` in the mongo and cache
backend suites. This led to a situation where the test changed in the
diff would fail if those tests happened to run before it! Luckily, the
side effect of the worker optimizations being enabled are not what cause
the unrelated failure, the test in this diff was a just a bit unaware of
the consequences of its mocking.
@maybe-sybr
Copy link
Contributor Author

To replicate the failure on master, you should be able to run:

$ tox -e 3.9-unit -- -k '(test_regression_worker_startup_info) or test_apply_simulates_delivery_info'

picking whatever toxenv fits your local python obviously :)

@maybe-sybr maybe-sybr changed the title test: Fix unexpected behaviour from bad mocking test: Fix unexpected failure from bad mocking Jun 8, 2021
@codecov
Copy link

codecov bot commented Jun 8, 2021

Codecov Report

Merging #6806 (652d2cc) into master (ce567e3) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6806      +/-   ##
==========================================
- Coverage   70.70%   70.69%   -0.01%     
==========================================
  Files         138      138              
  Lines       16601    16602       +1     
  Branches     2091     2091              
==========================================
  Hits        11737    11737              
- Misses       4668     4669       +1     
  Partials      196      196              
Flag Coverage Δ
unittests 70.69% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/backends/couchdb.py 37.31% <0.00%> (-0.57%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51634c3...652d2cc. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Jun 8, 2021

This pull request introduces 1 alert and fixes 2 when merging 652d2cc into 51634c3 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Module is imported with 'import' and 'import from'

@auvipy
Copy link
Member

auvipy commented Jun 8, 2021

can some more coverage be increased?

@maybe-sybr
Copy link
Contributor Author

maybe-sybr commented Jun 8, 2021

Not sure why the coverage has changed, especially in the couchdb source. IMO it's unrelated to this PR and maybe just codecov getting a bit confused. Hard to be sure with everything apparently being broken because of Fastly right now :/

Edit: yeah, looks like codecov is just a bit confused. It showed me a line from arangodb.py for some reason, and I can't even load the coverage diff for this MR at: https://app.codecov.io/gh/celery/celery/compare/6806 . It's probably fine

@auvipy auvipy merged commit 305851a into master Jun 9, 2021
@auvipy auvipy deleted the test/fix-request-stack-mocking branch June 9, 2021 03:22
@maybe-sybr maybe-sybr mentioned this pull request Jun 9, 2021
@auvipy auvipy added this to the 5.1.x milestone Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants