You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
Below is a video to show the problem. In this video:
I start with a test already in the code that succeeds (forgot to record it succeeding, sorry)
I change a spy to return a Promise that resolves to null, instead of its original data
I run the test and show it still succeeding (test compilation process is sped up FYI). The test should have failed in this step.
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
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!
The text was updated successfully, but these errors were encountered:
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:
null
, instead of its original dataexpect
method has a chance to run inside thethen
methodThis issue seems to appear whenever a test covers an asynchronous method and puts
expect
expression inside the callback to the chainedthen
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!
The text was updated successfully, but these errors were encountered: