Skip to content

Conversation

@BenHenning
Copy link
Member

@BenHenning BenHenning commented Oct 22, 2021

Explanation

Fix #3960

See #3960 for context, but two tests were regressed in #3796 due to an incorrect change in questions.json. This PR reverts the JSON issue, and corrects the two tests so that they match the expected outcomes pre-#3796.

Essential Checklist

  • The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

N/A -- test-only change

@BenHenning
Copy link
Member Author

@rt4914 & @vinitamurthi PTAL for codeowners.

@TheRealJessicaLi PTAL to make sure that this matches your suggestion for the recommended fix.

@oppiabot
Copy link

oppiabot bot commented Oct 25, 2021

Unassigning @vinitamurthi since they have already approved the PR.

Copy link
Contributor

@TheRealJessicaLi TheRealJessicaLi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@oppiabot
Copy link

oppiabot bot commented Oct 25, 2021

Unassigning @TheRealJessicaLi since they have already approved the PR.

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM

@rt4914 rt4914 merged commit b4e87cc into develop Oct 25, 2021
@rt4914 rt4914 deleted the fix-regressed-question-assessment-progress-controller-tests branch October 25, 2021 18:00
@BenHenning
Copy link
Member Author

BenHenning commented Oct 25, 2021

Thanks @rt4914 @vinitamurthi @TheRealJessicaLi !

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.

QuestionAssessmentProgressControllerTest regressed

5 participants