Fix #4334: Improved Test Cases Readability and Maintainability #4335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
October 21, 2023 16:31
In this commit, I've mad the following enhancements to the code: 1. Split the test-cases and move the test-case for checking the aria-disabled attribute to a separate it block. 2. Changed the use of the magic number 4 to the length of the input array excludeTimes for better flexibility and maintainability. Closes Hacker0x01#4334
In this commit, I've made the following enhancements to the code: 1. Replaced the magic value 17 with meaning constant for clarity. 2. Instead of directly checking the disabledTimeItems.length to be 2, make sure whether it's really the value we assigned (17 or 5pm). 3. Split the test-cases and move the test-case for checking the aria-disabled attribute to a separate it block. Closes Hacker0x01#4334
In this commit, I've made the following enhancements to the code: 1. Change the test case of finding the disabled time units count to the finding the enabled time units as the module involves on the existence of the includeTimes 2. Moved the test case to find the enabled time units through aria-disabled attribute to the separate it block 3. Replaced the magic value 45 with the actual computation which resolves to the magic value, to make it more maintainable and readable Closes Hacker0x01#4334
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ This pull request was sent to the PullRequest network.
@balajis-qb you can click here to see the review status or cancel the code review job.
Codecov Report
@@ Coverage Diff @@
## main #4335 +/- ##
=======================================
Coverage 96.63% 96.63%
=======================================
Files 27 27
Lines 2375 2375
Branches 953 953
=======================================
Hits 2295 2295
Misses 80 80 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #4334
Summary
I have made changes to improve the maintainability of certain test cases mentioned in the issue and removed magic numbers and refactored it a bit. This enhancement aims to make the core more readable and maintainable.
Motivation and Benefits
Improving test cases maintainability and removing magic numbers helps in the following ways:
Files Updated
test/filter_times_test.test.jstest/filter_times_test.test.jstest/exclude_times_test.test.jsChanges Made