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

Added more tests #22

Merged
merged 1 commit into from
Apr 19, 2021
Merged

Added more tests #22

merged 1 commit into from
Apr 19, 2021

Conversation

aliosman21
Copy link
Contributor

Added a few more tests:

  • Alpha lower & upper
  • Weekdays
  • Floating points
  • Release Versions
  • Sizes
  • Months

Considered adding sort by currency where
5euross > 5$ but thought of it as an overkill

Copy link
Owner

@LeeWannacott LeeWannacott left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! 💯 . I think I need to change the testing table to allow different lengths of arrays, i.e not limited to five. Another-words make the table dynamically from how many items are in the array and set classes maybe some other way.


test("Sizes", () => {
expect(createTestTable(["xs", "lg", "sm", "md", "xlg"])).toStrictEqual([
Copy link
Owner

Choose a reason for hiding this comment

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

I should make the testing table dependent on the size of the array passed to createTestTable; so we can have more sizes included rather than being limited to just five. Currently table-sort-js doesn't sort sizes; This could possibly be handy for eCommerce/Clothing websites.

});

test("Months", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

table-sort-js doesn't currently sort months shouldn't be too hard to add in though. Ⓜ️

});

test("Release Versions", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, release version is a common thing that would be sorted. 👍

});

test("Weekdays (Expects week begins at Monday)", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

table-sort-js doesn't currently sort Weekdays shouldn't be too hard to add in though.


// toBe for primitives like strings, numbers or booleans for everything else use toEqual(object)

test('Alpha - Capitalized ', () => {
expect(createTestTable(['Echo','Alpha','Bravo','Charlie','Delta'])).toStrictEqual(['Alpha','Bravo','Charlie','Delta','Echo']);
test("Alpha - Capitalized ", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Changing to the vertical format makes sense. I don't have experience with setting up tests, well I guess I do now. 🧪

@@ -1,35 +1,135 @@
const createTestTable = require('./table');
Copy link
Owner

Choose a reason for hiding this comment

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

Using "over ' is probably personal preference; but sure let's go with " from now on.

@LeeWannacott LeeWannacott merged commit 397baaf into LeeWannacott:master Apr 19, 2021
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.

None yet

2 participants