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

[BUG]: 'Expect' expressions in some tests of async operations are not running #20317

Open
mgporter opened this issue May 15, 2024 · 0 comments
Open
Labels
bug Label to indicate an issue is a regression

Comments

@mgporter
Copy link

Describe the bug

While working on a PR, I discovered that some asynchronous frontend tests would succeed, even when they should have failed. This particular behavior was found in core/templates/pages/contributor-dashboard-page/services/contribution-and-review.service.spec.ts, however it may be present in other tests as well.

URL of the page where the issue is observed.

core/templates/pages/contributor-dashboard-page/services/contribution-and-review.service.spec.ts

Steps To Reproduce

Below is a video to show the problem. In this video:

  1. I start with a test already in the code that succeeds (forgot to record it succeeding, sorry)
  2. I change a spy to return a Promise that resolves to null, instead of its original data
  3. I run the test and show it still succeeding (test compilation process is sped up FYI). The test should have failed in this step.
  4. Then I try out one fix to the problem: I change the function to async and add an await so that the expect method has a chance to run inside the then method
  5. I run the test again and show how it now fails (as it should)

This issue seems to appear whenever a test covers an asynchronous method and puts expect expression inside the callback to the chained then method, without returning a Promise or using await. The Jasmine documentation for async functions does not show tests done in this way, and it appears to not work.

Expected Behavior

As noted above, changing the spy to return a Promise that resolves to null should have caused the test to fail in step 3.

Screenshots/Videos

Oppia-Async-Test-Bug.mp4

What device are you using?

Desktop

Operating System

Linux

What browsers are you seeing the problem on?

No response

Browser version

No response

Additional context

Once we establish how to modify the tests so that they run correctly, I would be willing to work on the changes.

Tips for developers

Before addressing the bug, please identify which PR caused the issue (you can follow the steps here). If you identify the PR, comment on the issue with a link to it. If not, mention the commit hash of the oldest commit you saw the bug on (and the month and year it was made in).

Then, please leave a comment with details of the approach that you plan to take to fix the issue (see example).

Note: If this is your first Oppia issue, please make sure to follow our guidelines for choosing an issue and setting things up. You will also need to show a demo of the fix working correctly on your local machine. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label to indicate an issue is a regression
Projects
Status: Todo
Development

No branches or pull requests

2 participants