stargazed
stargazed copied to clipboard
abhijithvijayan
add unit tests
Checklist
- [x] Initial barebone
- [ ] Add unit tests for the commands offered by the cli
I was able to get Jest working with some Babel, the tests themselves I am still working on since I have two other projects I am working on right now too. :)
On Sat, Oct 12, 2019 at 9:32 AM Abhijith Vijayan [email protected] wrote:
@JacobMGEvans https://github.com/JacobMGEvans How is that PR coming ?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/abhijithvijayan/stargazed/issues/19?email_source=notifications&email_token=AGP4EOHYV5623PVVYNHTRE3QOHNYTA5CNFSM4JAARLQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBCAWCI#issuecomment-541330185, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGP4EODXHIDWMNRGI6OHQE3QOHNYTANCNFSM4JAARLQQ .
I did add exports to a lot of the functions in the file, but those can easily be removed when determined which ones are not needed for isolated testing.
I was also trying to get a feel for the codebase/the flow of data and how to approach testing it.
@JacobMGEvans I think a lot of code refactoring is needed here and there in the source code. This was all written very soon so didn't get time to refactor.
Well, tests will definitely help when you go to refactor because you can just run watch on them and make sure the tests don't break :)
I try to write tests so the code implementation doesn't matter i.e. refactors dont break them... the only thing that matters is the input => output and behaviors remain the same/consistent.
@JacobMGEvans You want me to assign this to someone else?
Refactor seems to be quite needed as CLI hit 2700+ weekly downloads. :grinning:
Wow! Congratulations, definitely a good idea to refactor for performance then!
You can assign other people, I probably will still write tests soon regardless. I am just finishing up with another issue from a different OSS and a few from my own side project.
Also I really like your project and how you have communicated, it's been a pleasant experience. So I certainly would like to assist 😄
@JacobMGEvans Thanks a lot for that.
It's not everyday someone helps in such tiny OSS projects. Glad that you are ready to contribute. Appreciate it.
Last night I started focusing on the unit tests, I am having issues importing the functions and statements for some reason... Might need to spend a little more time on the jest configuration, not sure what's the exact problem yet.
lmao, I am certain I can figure it out I just wanted to keep you updated 😆
@abhijithvijayan Alright, I got this mostly figure out it was not configuration issues. The fact you were using a module.exports on an anonymous async function was making is impossible to test... I have some solutions,
-
firstly I am going to make the test work with the code as-is then make the PR
-
I am going to refactor it to use ES6 modules
import/exportwhich export default should would the same as how you havemodule.exportswith the testing in place we can refactor with confidence it will keep working as expected. -
Once the import/export is in place testing specific functions will be far easier and refactoring the code base will be far easier.
-
Integration and E2E(likely not needed) will be the next endeavor
@JacobMGEvans I am open to any modifications as long as the project works and code quality is improved.
@abhijithvijayan Interesting enough if I try to pass false into the options that are primitives is throws an error, if I pass true it works fine... I might open a bug issue a little bit
Would you be able to give me some mock or actual payloads and responses? I would rather not go through the whole process if you have anything like that leftover from developing it.
@JacobMGEvans I have fixed the export issue and also added a sample mock payload to the test file.
Now you can continue adding tests.
Huh, it was working fine when I tested it locally... I use babel-node though 😬 lol
Glad you got it fixed though Ill check out the changes and continue with testing.
Someone also uncommented out tests that I had TODOs on because they were incomplete and a comment was added that "it doesnt return anything" which is why there was a TODO there to finish the test...
@JacobMGEvans I was trying out the error fixes. Feel free to work on it.
@JacobMGEvans Is babel config and its packages needed to run the tests?
Yes.
On Thu, Oct 17, 2019, 10:37 PM Abhijith Vijayan [email protected] wrote:
@JacobMGEvans https://github.com/JacobMGEvans Is babel config and its packages needed to run the tests?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/abhijithvijayan/stargazed/issues/19?email_source=notifications&email_token=AGP4EODENU2T2DTN6ANWYQTQPEVNZA5CNFSM4JAARLQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBSNA4I#issuecomment-543477873, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGP4EOGKAG4QG7KPBDAMQVLQPEVNZANCNFSM4JAARLQQ .
So far it's 50% coverage. Some of the functions and behavior are difficult to test due to how errors are handled and also readme is written locally... So that can overwrite the local repos readme when running parts of the code in tests.
On Sun, Oct 20, 2019, 11:45 AM Abhijith Vijayan [email protected] wrote:
@JacobMGEvans https://github.com/JacobMGEvans The tests are almost complete right!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/abhijithvijayan/stargazed/issues/19?email_source=notifications&email_token=AGP4EOBC5YN2YAFYTZMMISLQPSDJ5A5CNFSM4JAARLQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBYOJAY#issuecomment-544269443, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGP4EOG64OTXULIDQ7FOWU3QPSDJ5ANCNFSM4JAARLQQ .
@JacobMGEvans not really.
CLI creates README.md, and local file is readme.md
If you have tried the cli within the folder, you can see it doesn't affect the original readme.md file
I wasn't asking a question I was stating what happens when you run the function in the Jest test, also it is not cap sensitive, Node just writes to the readme.md, like I said.
I thought since the command Node cli.js -u "USERNAME" creates README.md along with readme.md, it wouldnt be a problem too, while testing.
Maybe renaming current readme.md to just readme might help right!
Perhaps. I haven't been able to get to it for a few days.
On Sun, Oct 20, 2019, 10:59 PM Abhijith Vijayan [email protected] wrote:
I thought since the command Node cli.js -u "USERNAME" creates README.md along with readme.md, it wouldnt be a problem too, while testing.
Maybe renaming current readme.md to just readme might help right!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/abhijithvijayan/stargazed/issues/19?email_source=notifications&email_token=AGP4EOE2YOUL55WMKTMAIHDQPUSJFA5CNFSM4JAARLQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBY7MYQ#issuecomment-544339554, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGP4EOGFRX2XVF3PJ5Y2TZTQPUSJFANCNFSM4JAARLQQ .