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.

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

fix: Add tests for the various parsers and cover edge cases #947

Closed
wants to merge 11 commits into from

Conversation

lbrooks
Copy link
Contributor

@lbrooks lbrooks commented Nov 15, 2024

List of updates to the method:

  • Sanitize numbers to ensure within [0-255]
    • When called with isRgbw == false, numbers outside of [0-255[ could make it into the PaletteType[] returned
  • When there aren't enough values to complete the last color, fill the missing values with 0s
    • In the previous code, the value would contain NaN
  • Simplify logic
    • Switched from a reducer needing state to a simple for loop
  • Add tests to cover all edge cases

* Sanitize numbers to ensure within [0-255]
* Gracefully handle incorrect number of values
* Simplify logic
* Add tests to cover all edge cases

Signed-off-by: Lawrence Brooks <[email protected]>
Signed-off-by: Lawrence Brooks <[email protected]>
@lbrooks lbrooks marked this pull request as ready for review November 15, 2024 05:14
@lbrooks lbrooks changed the title fix: Add tests for parsePaletteRaw, cover edge cases fix: Add tests for colormap, keymap, palette, superkey parsers and cover edge cases Nov 15, 2024
@lbrooks lbrooks changed the title fix: Add tests for colormap, keymap, palette, superkey parsers and cover edge cases fix: Add tests for the various parsers and cover edge cases Nov 15, 2024
@alexpargon
Copy link
Contributor

Hey, @lbrooks !! Thanks for your contribution!

I have been adding tests to cover better all the possible macro scenarios.

With the expanded coverage using a backup as a data source, the tests fail with the refactored function, so, could you check it?

test-palette-parse

@lbrooks
Copy link
Contributor Author

lbrooks commented Feb 7, 2025

I'm closing this one as it's gone through a number of updates and depends on fixing the Focus.unit.ts file (ignoring the test case due to the change in how SerialPort is imported).

I will cherry pick the tests and refactoring in this PR to another PR which doesn't have the additional noise.

@lbrooks lbrooks closed this Feb 7, 2025
@lbrooks lbrooks deleted the test-palette-parse branch February 7, 2025 19:39
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.

2 participants