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

Add test to HintAndSolutionManagerService to check that hints aren't released after a correct answer is submitted. #4585

Merged
merged 4 commits into from
Jan 23, 2018

Conversation

tjiang11
Copy link
Contributor

Add test for #4579

@tjiang11 tjiang11 requested a review from seanlip January 21, 2018 04:29
@codecov-io
Copy link

codecov-io commented Jan 21, 2018

Codecov Report

Merging #4585 into develop will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #4585      +/-   ##
==========================================
+ Coverage    44.79%   44.8%   +<.01%     
==========================================
  Files          382     382              
  Lines        23211   23212       +1     
  Branches      3731    3731              
==========================================
+ Hits         10398   10400       +2     
+ Misses       12813   12812       -1
Impacted Files Coverage Δ
...ploration_player/HintsAndSolutionManagerService.js 77.66% <0%> (+0.97%) ⬆️
...dev/head/services/SpeechSynthesisChunkerService.js 23.61% <0%> (+1.07%) ⬆️

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 7b2fb13...bfeb115. Read the comment docs.

Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Thanks @tjiang11! Couple notes.

$timeout.flush();

// Because a correct answer was submitted, the next hint should not be
// available.
Copy link
Member

Choose a reason for hiding this comment

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

Are there three hints? The format of the test implies there are only two.

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 think there are two

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, I think I misread. Looks good!

expect(hasms.isHintViewable(0)).toBe(true);
expect(hasms.isHintViewable(1)).toBe(false);
expect(hasms.isSolutionViewable()).toBe(false);
expect(hasms.isSolutionViewable()).toBe(false);
Copy link
Member

Choose a reason for hiding this comment

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

Why the duplicate line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@seanlip seanlip merged commit 94c3c83 into oppia:develop Jan 23, 2018
giritheja added a commit to giritheja/oppia that referenced this pull request Jan 23, 2018
* upstream/develop: (77 commits)
  Add test to HintAndSolutionManagerService to check that hints aren't released after a correct answer is submitted. (oppia#4585)
  Chance cancel property in SpeechSynthesisChunkerService into its own variable. (oppia#4591)
  Enable indent for func-expr (oppia#4588)
  Update text of refresher exp modal. (oppia#4584)
  Update content-type for JSON responses. (oppia#4583)
  Prevent hints from being released after correct answer is submitted. (oppia#4580)
  Change fraction placeholder when no integer part is allowed. (oppia#4581)
  Fix oppia#4567: Changed condition to check range enclosed criteria, edited existing test (oppia#4575)
  Fix part of oppia#3905: Added semi-rule (oppia#4576)
  Adds proper message for validation error in skill ID. (oppia#4577)
  Fix part of oppia#3905: Add checks for correct docstring style. (oppia#4572)
  Fix part of oppia#4374: Add docstrings to core.domain.collection_domain. (oppia#4569)
  Add check to prevent 'SpeechSynthesisUtterance is undefined' error. (oppia#4571)
  Fix of oppia#2533: Modified the stripFormatting filter to preserve newlines in RTE cut-and-paste (oppia#4568)
  Fix part of oppia#3905: Add check for console.log statement (oppia#4564)
  Change Java prerequisite to v8 (oppia#4570)
  Fix oppia#1542: RTE Components correctly displayed in answer group header (oppia#4565)
  Fix part of oppia#4374: Add docstrings to core.domain.param_domain (oppia#4551)
  Fix oppia#4555: Address review comments for oppia#4536. (oppia#4556)
  Update changelog, AUTHORS, CONTRIBUTORS and credits. (oppia#4560)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants