Adding testing infrastructure

[Immutablevoid]

This pull request sets up the testing infrastructure using Jest .

To run the test : npm test
I only created one simple test as a starting point. The test does check if the Redis server is open.

NOTE: npm test will most likely not end correctly with error:
"Jest did not exit one second after the test run has completed. This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with --detectOpenHandles to troubleshoot this issue"

(CTRL + C to exit test.)

This is because the Redis server is still online, closing the server after testing would fix the issue.
If there is a way to handle opening and closing Redis inside the project, this problem won't happen anymore <- CREATE NEW ISSUE TO SOLVE THIS

[cindyorangis]

I ran this test on my machine and it worked great. My first test failed because I was trying to open redis in a port that was already occupied. I killed the instance using the port and tried again and the second test was successful as expected.

[cindyorangis]

I just realized there's already a npm test script in the package.json file, be careful when you merge that you don't delete the test script that's already there

[Immutablevoid]

I just realized there's already a npm test script in the package.json file, be careful when you merge that you don't delete the test script that's already there

Yup. Thanks, Ill rebase this pull request later today when I have time.

[Immutablevoid]

@UltimaBGD I had to merge your ESLint pull request with my Jest testing infrastructure pull request. It would be great if you can look it over and give an approval. I changed npm test to call Jest instead of ESLint. ESLint can still be called with npm run eslint and npm run eslint-fix.

[humphd]

@ImmutableBox what do you mean by "I had to merge with your eslint pr"? You don't want to do that. If you need to test, that's fine locally, but you should rebase on it when it gets landed.

[humphd]

To confirm: that PR has landed, so you can just rebase on master, don't merge.

[Immutablevoid]

To confirm: that PR has landed, so you can just rebase on master, don't merge.

Yes, I rebase to master. I then used git merge master with the branch I was working on and fixed merging conflicts on package.json and package-lock.json. When I ran npm run eslint-fix it seemed to fix the formatting for all the other files.

This is what my git log looks like:

Where Adding testing infrastructure #42 is my first commit, Add ESLint with Airbnb style is what's currently on master, and the commit I just uploaded.

If this is wrong I can do it again.

[humphd]

OK, so this is wrong. You've done a merge, and we don't want to merge while on a PR branch, only rebase. You want to update your commits so they are in line with the latest master, but not merge with it. The merge only happens when the PR gets approved and landed.

We can fix this together in class tomorrow if you want, or you can do it yourself. Basically, you want to rebase your 8b5731bf61ce781088a83ebcd522638527824d87 commit on master, and fix the conflicts to produce something similar to what your merge does. Then you need to git push origin issue42 -f.

[UltimaBGD]

You should be able to run Jest and ESlint in the same command with the "&&" operators, is there a reason why ESlint had to be taken out of the test?

[Immutablevoid]

You should be able to run Jest and ESlint in the same command with the "&&" operators, is there a reason why ESlint had to be taken out of the test?

No, not really just following this blog he had it separated. Ill add it back in.

[humphd]

@UltimaBGD the && operator won't work on all shells (e.g., many Windows shells). There are few work arounds for this:

1. we can use pre and post scripts to run things before/after other steps, https://docs.npmjs.com/misc/scripts
2. we could use https://www.npmjs.com/package/npm-run-all to allow for a set of steps

In general, we have to be careful that our scripts work on Windows, since so many common tricks like this will fail :(

[UltimaBGD]

Okay, I think a pre script would make sense for this. How do we want to go about adding that in? Get this merged as it is and then I make another PR with the prescript?

[humphd]

Given that nothing is depending on these tests yet, or even eslint for that matter, let's get this rebased (package* files need updating) and then do more PRs on top.

[UltimaBGD]

Alright, I will keep my eye on it and work on that after.

[Immutablevoid]

Given that nothing is depending on these tests yet, or even eslint for that matter, let's get this rebased (package* files need updating) and then do more PRs on top.

I think I got it down.

git log:

I think I still have to make another commit or rebase, since Jest doesn't work anymore.

[humphd]

Yes, that looks right, good work. Just do whatever you have to in order to get Jest to work again, and then do git commit --amend --no-edit and update your current commit, then git push origin issue42 -f when you are ready to have us review.

[Immutablevoid]

OK. Jest is now working with npm test, ill create a new issue for running both ESLint and Jest together with the comments provided.

Like before I ran npm run eslint and npm run eslint-fix so it seem to fix the spacing for the rest of the files.

[humphd]

One thing I notice.

[humphd]

Leave the test script alone, and instead add one for jest:

"test": "npm run eslint",
"jest": "jest",

In the new bug you file we can combine these via a call to "pretest": "npm run eslint" and change test to run jest

[Immutablevoid]

Fixed in c3d8a6a.

[humphd]

The particular test we have here is not going to be one we want to keep long term, but I know that wasn't your goal. I say we take this and keep iterating on it in other PRs.

Great work, thanks for taking this on!

[UltimaBGD]

Looks good to me!