Fix #4689: Add regex check to prohibit post() and postDelayed() #4900

kkmurerwa · 2023-03-14T10:55:28Z

Explanation

Fix #4689: When merged, this PR will;

Add regex check to prohibit the usage of post() and postDelayed()
Add tests for the regex check added.
Add file exemptions for files that don't meet the new rules.

The commits pushed on 4th April 2023 will;

Remove tests from exemptions list of the regex check.
Add RegexPatternValidationCheckTest.kt to exemptions since it is matched correctly against the regex. This is an undesired side effect or removing tests from exemptions.
Modify the language of the error message to align with Oppia guidelines.
Remove the SpotlightFragment.kt from exemptions since it contains no instance of post() or postDelayed()
Update the regex to take care of cases with curly braces such as post {} instead of only matching post() and postDelayed().
Cater for any spaces between the post/postDelayed and the bracket used i.e. post (), postDelayed () and post {}.
Add another test to verify check for usages of post with curly braces.
Add an extra test in the test methods to ensure that the correct error message is being shown.

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

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)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

- Add regex check to prohibit the usage of post() and postDelayed() - Add tests for the regex check added

…xemptions

oppiabot · 2023-03-21T17:46:19Z

Hi @kkmurerwa, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

kkmurerwa · 2023-03-22T12:14:14Z

Hey @BenHenning. I did some housekeeping here and merged the latest changes from develop. All checks were passing before the merge and should even after the merge. The failing checks just need a rerun.

BenHenning

Thanks @kkmurerwa! Had some follow-up thoughts, PTAL.

scripts/assets/file_content_validation_checks.textproto

scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt

…ther error message displayed is correct

kkmurerwa · 2023-04-04T09:40:04Z

PTAL @kkmurerwa

… instead of post()

kkmurerwa · 2023-04-04T15:01:26Z

Hey @BenHenning. I updated this PR with some new changes. They are detailed in a second section of the PR comment labeled Latest commit will. I have also addressed all issues mentioned and I hope it is now good to go.

kkmurerwa · 2023-04-04T15:01:39Z

PTAL @BenHenning

- Refactor the prohibited regex to cover false positives in some files - Remove the wrongly exempted SpotlightFragment.kt file

kkmurerwa · 2023-04-05T09:56:44Z

Hey @BenHenning. A few checks failed after the last commit but it shouldn't be a problem. A quick rerun should fix it.

MohitGupta121 · 2023-04-05T10:05:08Z

@kkmurerwa Done, I re-run the CI checks. Thanks.

kkmurerwa · 2023-04-05T10:09:31Z

Thanks @MohitGupta121

BenHenning

Thanks @kkmurerwa. Took another pass--please make sure to respond to all comments before sending PRs back into review.

scripts/assets/file_content_validation_checks.textproto

scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt

scripts/assets/file_content_validation_checks.textproto

- Rearrange the exempted files n lexicographical order. - Rename the doesNotUsePostorPostDelayed to doesNotUsePostOrPostDelayed - Modify the failure message to fix grammatical errors

kkmurerwa · 2023-04-06T10:19:20Z

@BenHenning I have responded to all unresolved comments and fixed the issues highlighted. I believe it is ready for another review.

kkmurerwa · 2023-04-06T10:19:42Z

PTAL @BenHenning

oppiabot · 2023-04-06T10:22:46Z

Unassigning @kkmurerwa since a re-review was requested. @kkmurerwa, please make sure you have addressed all review comments. Thanks!

BenHenning

Thanks @kkmurerwa! This LGTM!

chore: This it will;

48346fe

- Add regex check to prohibit the usage of post() and postDelayed() - Add tests for the regex check added

kkmurerwa requested a review from BenHenning as a code owner March 14, 2023 10:55

chore: Add more files with instances of post() and postDelayed() to e…

591fb2a

…xemptions

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Mar 21, 2023

Merge branch 'develop' into regex-to-prohibit-post-and-post-delayed

9ede968

oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Mar 22, 2023

Merge branch 'develop' into regex-to-prohibit-post-and-post-delayed

38de61d

BenHenning self-assigned this Mar 24, 2023

BenHenning requested changes Mar 29, 2023

View reviewed changes

BenHenning assigned kkmurerwa and unassigned BenHenning Mar 29, 2023

kkmurerwa added 3 commits April 3, 2023 10:45

Merge branch 'develop' into regex-to-prohibit-post-and-post-delayed

44f787a

feat: Remove Test files from exemptions and add new test to check whe…

ca44f2f

…ther error message displayed is correct

fix: Fix lint check issues

cc0ad0e

kkmurerwa removed their assignment Apr 3, 2023

kkmurerwa added 2 commits April 4, 2023 16:51

feat: Modified regex to cater for cases where post is used as post {}…

00ae091

… instead of post()

test: Add test for new curly braces condition for post {}

b2acbbc

oppiabot bot assigned BenHenning Apr 4, 2023

This commit will;

aeffc8e

- Refactor the prohibited regex to cover false positives in some files - Remove the wrongly exempted SpotlightFragment.kt file

BenHenning requested changes Apr 6, 2023

View reviewed changes

BenHenning assigned kkmurerwa and unassigned BenHenning Apr 6, 2023

kkmurerwa added 2 commits April 6, 2023 11:41

This commit ;

cfaf681

- Rearrange the exempted files n lexicographical order. - Rename the doesNotUsePostorPostDelayed to doesNotUsePostOrPostDelayed - Modify the failure message to fix grammatical errors

Merge branch 'develop' into regex-to-prohibit-post-and-post-delayed

129283f

oppiabot bot assigned BenHenning and unassigned kkmurerwa Apr 6, 2023

BenHenning approved these changes Apr 11, 2023

View reviewed changes

BenHenning merged commit 0baeeb1 into oppia:develop Apr 11, 2023

kkmurerwa deleted the regex-to-prohibit-post-and-post-delayed branch May 10, 2023 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4689: Add regex check to prohibit post() and postDelayed() #4900

Fix #4689: Add regex check to prohibit post() and postDelayed() #4900

kkmurerwa commented Mar 14, 2023 •

edited

Loading

oppiabot bot commented Mar 21, 2023

kkmurerwa commented Mar 22, 2023

BenHenning left a comment

kkmurerwa commented Apr 4, 2023

kkmurerwa commented Apr 4, 2023

kkmurerwa commented Apr 4, 2023

kkmurerwa commented Apr 5, 2023

MohitGupta121 commented Apr 5, 2023

kkmurerwa commented Apr 5, 2023

BenHenning left a comment

kkmurerwa commented Apr 6, 2023

kkmurerwa commented Apr 6, 2023

oppiabot bot commented Apr 6, 2023

BenHenning left a comment

Fix #4689: Add regex check to prohibit post() and postDelayed() #4900

Fix #4689: Add regex check to prohibit post() and postDelayed() #4900

Conversation

kkmurerwa commented Mar 14, 2023 • edited Loading

Explanation

Essential Checklist

For UI-specific PRs only

oppiabot bot commented Mar 21, 2023

kkmurerwa commented Mar 22, 2023

BenHenning left a comment

Choose a reason for hiding this comment

kkmurerwa commented Apr 4, 2023

kkmurerwa commented Apr 4, 2023

kkmurerwa commented Apr 4, 2023

kkmurerwa commented Apr 5, 2023

MohitGupta121 commented Apr 5, 2023

kkmurerwa commented Apr 5, 2023

BenHenning left a comment

Choose a reason for hiding this comment

kkmurerwa commented Apr 6, 2023

kkmurerwa commented Apr 6, 2023

oppiabot bot commented Apr 6, 2023

BenHenning left a comment

Choose a reason for hiding this comment

kkmurerwa commented Mar 14, 2023 •

edited

Loading