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.

Sign up for GitHub

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

Jump to bottom

Add tests for FractionInputInteractionView #4135

Closed
BenHenning opened this issue Jan 27, 2022 · 5 comments · Fixed by #5224 or #5321
Closed

Add tests for FractionInputInteractionView #4135

BenHenning opened this issue Jan 27, 2022 · 5 comments · Fixed by #5224 or #5321
Assignees
@masclot
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

FractionInputInteractionView should have dedicated tests to verify that it behaves correctly as an interaction. Today, it's tested through InputInteractionViewTestActivity, but these tests ought to be moved to a dedicated test suite for fraction input, and other fraction-specific tests should be added.
@bhaktideshmukh
Copy link
Contributor

bhaktideshmukh commented Feb 2, 2022 •
edited
Loading

@BenHenning Hey can I work on this?
And also please do suggest which PR i should consider as reference to solve this issue.

@BenHenning
Copy link
Member Author

Sure @bhaktideshmukh. For reference, you can look at InputInteractionViewTestActivity to see how we currently set up the fraction tests, though that suite isn't a good example of well-written tests themselves. For those, I suggest looking at the few tests in StateFragmentTest and StateFragmentLocalTest that verify fractions functionality (though a dedicated FractionInputInteractionViewTest should test all aspects of the fraction input interaction). I suggest writing an initial test suite and then following up with a draft PR where we can provide additional suggestions.

@bhaktideshmukh bhaktideshmukh mentioned this issue Mar 10, 2022
Closed
6 tasks
@BenHenning
Copy link
Member Author

Note also that #2173 adds a new MathExpressionInteractionsViewTest that can probably serve as a good reference for future interaction test suites (I haven't looked at your PR yet @bhaktideshmukh it may be worth looking at MathExpressionInteractionsViewTest and seeing if anything from that test suite is worth basing changes to the new FractionInputInteractionViewTest).

@bhaktideshmukh bhaktideshmukh mentioned this issue Mar 28, 2022
Closed
6 tasks
@bhaktideshmukh bhaktideshmukh mentioned this issue Apr 27, 2022
Closed
6 tasks
@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jun 13, 2022
@Broppia Broppia added dev_team and removed user_team labels Aug 2, 2022
@BenHenning BenHenning added Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. labels Sep 16, 2022
@seanlip seanlip added enhancement End user-perceivable enhancements. and removed issue_type_infrastructure labels Mar 28, 2023
@adhiamboperes adhiamboperes added the Work: Low Solution is clear and broken into good-first-issue-sized chunks. label Aug 14, 2023
@adhiamboperes adhiamboperes added this to the 1.0 Global availability milestone Oct 1, 2023
@adhiamboperes adhiamboperes added Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. and removed Work: Low Solution is clear and broken into good-first-issue-sized chunks. Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. labels Oct 9, 2023
@adhiamboperes adhiamboperes mentioned this issue Dec 5, 2023
Merged
6 tasks
BenHenning pushed a commit that referenced this issue Jan 9, 2024
@masclot @adhiamboperes
Fix #4135, Fix part of #5070: In FractionInteraction UI, leave submit…
6723268 
… button enabled when answer is empty. (#5224)

Fix part of #5070: In
FractionInteraction UI, leave submit button enabled when answer is
empty. Show an error on submitting an empty answer. The error message
already exists and is the same as in
oppia/oppia#18379.

Demo video:
[leave_submit_button_enabled_on_empty_answer_v3.webm](https://github.com/oppia/oppia-android/assets/103062089/d072ae88-c462-455c-a324-57680d4a82c5)

The new error messages for empty inputs on submit are listed here:
[oppia/design-team#71(comment)](oppia/design-team#71 (comment))

I added an accessibility-label exemption for
FractionInputInteractionViewTestActivity as this activity is only used
in tests.

Fix #4135: incidentaly, this change also fixes #4135, since I had to
split the tests for FractionInputInteraction

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <[email protected]>
@github-actions github-actions bot reopened this Jan 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2024

The issue is reopened because of the following unresolved TODOs:

oppia-android/app/src/main/java/org/oppia/android/app/customview/interaction/FractionInputInteractionView.kt

Line 23 in 6723268

// TODO(#4135): Add a dedicated test suite for this class.

@adhiamboperes
Copy link
Collaborator

@masclot, please create a follow up PR to remove this leftover TODO and close the issue.

@masclot masclot mentioned this issue Jan 25, 2024
Merged
6 tasks
adhiamboperes added a commit that referenced this issue Jan 26, 2024
@masclot @adhiamboperes
Fix #4135: Remove leftover TODO (#5321)
f5be6f6 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fix #4135: The only thing left is removing the TODO comment, as the
tests were created in #5224

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Co-authored-by: Adhiambo Peres <[email protected]>
@adhiamboperes adhiamboperes removed this from the 1.0 Global availability milestone Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@masclot masclot

Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone
7 participants
@seanlip @BenHenning @adhiamboperes @bhaktideshmukh @aayushimathur6 @masclot @Broppia