Fix: tomorrow never comes #364
Conversation
…yOrg#293 Refactored AlarmController to move logic that causes bug to separate testable function with optional parameter for passing in currentDayMinutes Refactored updateNonRecurringAlarmDay in Constants.kt with overloaded function, allowing testability with optional parameter currentDayMinutes Added unit test class AlarmControllerTest.kt under app/src/test folder Added readme explaining how to run tests
|
@naveensingh In this phase I have added unit tests to reproduce the issue which causes the build to fail intentionally. More tests could be added to verify different scenarios as described in the original issue report. If you are happy with this approach I can then push the fix which will then cause the build to succeed and pass the test. Otherwise if you are not happy with adding unit tests to reproduce the issue then I will create a separate pull request with just the fix. |
naveensingh
left a comment
naveensingh
left a comment
There was a problem hiding this comment.
Thanks, but there are three problems:
- The code isn't even testing the right thing. Making that test pass will be worse.
- Obviously AI-generated (see guidelines).
- I'm already working on this.
Continued use of AI tools to generate proposals, arguments and pull requests that don't meaningfully help the project or waste time will result in moderation.
|
1.) You are completely wrong and as usual with your comments, unjustified without giving anyone a chance to actually respond. This does help the project as it sets up unit tests (which you don't have) to test the functionality that results in the described bug in the issue. Also this is a DRAFT PR, which you could feedback on to improve. 2.) The code was not AI generated. I used copilot to help annotate comments only. 3.) How does extracting unit tests to reproduce the issue not result in a better way forward? |
|
Literally all this code does currently is slightly refactors the existing area which results in the bug described in order to make the functions testable. Adds unit tests (which can be improved to reproduce the exact state that results in the bug described). Yet you somehow have a problem with that because you assume everything AI generated when it isn't. I use AI to fill in forms, which I have seen you say yourself on other pull requests that you are "too lazy" to fill in. This is reinforced by your comment on this pull request fixing a spacing issue in the stop watch lap times: #363 All you state is that changing the width isn't the correct fix and close it. No alternative explanation or anything, it's frankly just unprofessional. The other pull request is very simple, fixes the issue and you provide no justification. |
|
@naveensingh Is there anyone else in your organisation that can provide an objective second opinion on this or is it what you say goes? |
2 participants
THIS IS CURRENTLY A DRAFT PULL REQUEST - Not yet completed
Type of change(s)
What changed and why
Fix for the "tomorrow never comes" bug where alarms set for tomorrow would incorrectly be rescheduled for the next day when the app was reopened (after device restart) when the alarm time had passed.
Phase 1: Testing infrastructure & bug reproduction - Completed
AlarmControllerreschedule logicAlarmController.rescheduleEnabledAlarms()to extract testableshouldRescheduleAlarm()functionupdateNonRecurringAlarmDay()accept optional time parameter for testingPhase 2: Bug fix - TODO
shouldRescheduleAlarm()to properly handle staleTOMORROW_BITvalues from databaseTests performed
./gradlew testFossDebugUnitTestBefore & after preview
N/A - internal logic fix with no UI changes
Closes the following issue(s)
Checklist
CHANGELOG.md(if applicable).