Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed CeleryTestCase and associated calling code. #3233

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

jathanism
Copy link
Contributor

Closes: #DNE

What's Changed

Now that we have CELERY_TASK_ALWAYS_EAGER = True in our test config, we no longer need this. Celery tasks/Jobs can be tested without the need for a single-threaded background worker so long as they are in a TransactionTestCase.

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

Now that we have `CELERY_TASK_ALWAYS_EAGER = True` in our test config, we no longer need this. Celery tasks/Jobs can be tested without the need for a single-threaded background worker so long as they are in a `TransactionTestCase`.
@bryanculver bryanculver added the type: housekeeping Changes to the application which do not directly impact the end user label Feb 6, 2023
Copy link
Contributor

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

@@ -1903,9 +1903,8 @@ def test_regex_validation(self):
cf.delete()


class CustomFieldBackgroundTasks(CeleryTestCase):
class CustomFieldBackgroundTasks(TransactionTestCase):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit curious about whether any of these test cases even need to be TransactionTestCases any more - any chance you looked into this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try that and they do require it.

@jathanism jathanism merged commit 40e0ad5 into next Feb 6, 2023
@jathanism jathanism deleted the jathanism-remove-celerytestcase branch February 6, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: housekeeping Changes to the application which do not directly impact the end user
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants