Skip to content

Conversation

@FirstLemon
Copy link
Contributor

Tried to add some tests for bit library

  • arshift
  • band
  • bnot
  • bor
  • bswap
  • bxor
  • lshift
  • rol
  • ror
  • rshift
  • robit
  • tohex

Copy link
Member

@sarahsturgeon sarahsturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give this a more thorough review when I free up, but on first glance, I'm impressed! 🚀

Thank you :) 🫶

Copy link
Member

@sarahsturgeon sarahsturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Two nits, I'll let you decide if you want to address them

  1. Prefer errWith( [[full error message]] ) over .err()
  2. Keep the test names consistent (are you going to prefix with "Should"? keep casing consistent, etc.)

Thanks again!

Comment on lines 36 to 38
expect( bit.arshift, nil, nil ).to.err()
expect( bit.arshift, "abc", "def" ).to.err()
expect( bit.arshift, {}, {} ).to.err()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Might be better to catch the full error message here

- fixed formatting issues
- hopefully made the names better
- corrected the tests a bit
@FirstLemon
Copy link
Contributor Author

Good call
I realized I can't name stuff but I attempted my luck in finding better names
And I swapped to.err() with to.errWith([[ errMsg ]]) and toNot.err() for some cases
Thanks so far!

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