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

[SDESK-7442] Fix and improve tests #2153

Merged
merged 4 commits into from
Dec 13, 2024
Merged

[SDESK-7442] Fix and improve tests #2153

merged 4 commits into from
Dec 13, 2024

Conversation

eos87
Copy link

@eos87 eos87 commented Dec 10, 2024

Purpose

To fix the tests that are related to EventsAsyncService.

What has changed

  • Added a helper method (clone_with) in EventResourceModel to facilitate the cloning and deep merging of a model instance and a given dict of updates (we can consider moving this base model later on if needed)
  • Adjusted EventsAsyncService create and update methods to match the same functionality, as the legacy service uses patch or post method to bypass the hooks. Instead, in the async service, I've abstracted the logic, which is bypassed in the legacy service, to explicitly use it so it is easier to follow and to understand.
  • Updated events_tests.py to use new EventsAsyncService
  • Improved test case so we don't have to repeat async with self.app.app_context(): all over the tests

What's pending

  • There are still references to old events service but we will move them as we move forward
  • prod_api tests are still broken but I will take a look at those in SDESK-7456
    SDESK-7442

Thanks for checking!

SDESK-7442
@eos87 eos87 added this to the 3.0 milestone Dec 10, 2024
@eos87 eos87 marked this pull request as ready for review December 11, 2024 14:34
@eos87 eos87 removed the WIP label Dec 11, 2024
@eos87 eos87 requested a review from MarkLark86 December 12, 2024 12:34
@eos87 eos87 merged commit 3bfad45 into async Dec 13, 2024
10 of 20 checks passed
@eos87 eos87 deleted the hg/SDESK-7442-fix-tests branch December 13, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants