adding tests for CLI functionality #1718

daemon1024 · 2020-10-06T15:13:13Z

Potentially closes #1694

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm run test-all
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
ask @publiclab/is-reviewers for help, in a comment below
Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

gitpod-io · 2020-10-06T15:13:17Z

codecov · 2020-10-06T15:16:41Z

Codecov Report

Merging #1718 into main will decrease coverage by 1.23%.
The diff coverage is 38.63%.

@@            Coverage Diff             @@
##             main    #1718      +/-   ##
==========================================
- Coverage   66.67%   65.43%   -1.24%     
==========================================
  Files         130      133       +3     
  Lines        2686     2743      +57     
  Branches      433      439       +6     
==========================================
+ Hits         1791     1795       +4     
- Misses        895      948      +53

Impacted Files	Coverage Δ
src/Modules.js	`100.00% <ø> (ø)`
src/modules/EdgeDetect/Module.js	`100.00% <ø> (ø)`
src/modules/WebglDistort/Module.js	`2.29% <0.00%> (ø)`
src/modules/ColorHalftone/Module.js	`3.63% <3.63%> (ø)`
src/modules/ColorHalftone/index.js	`100.00% <100.00%> (ø)`
src/modules/Colorbar/Module.js	`100.00% <100.00%> (ø)`
src/modules/Crop/Module.js	`88.88% <100.00%> (ø)`
src/modules/EdgeDetect/EdgeUtils.js	`86.81% <100.00%> (-0.15%)`	⬇️
src/modules/Resize/Module.js	`100.00% <100.00%> (ø)`
src/modules/Resize/Resize.js	`100.00% <100.00%> (ø)`
... and 3 more

daemon1024 · 2020-10-06T15:17:49Z

This is a very simple implementation of what I could perceive.

To test it out you can run

./index.js test

We could test out individual functions like this.
What do you think about it @jywarren @publiclab/is-reviewers @publiclab/image-sequencer-guides

jywarren · 2020-10-06T16:03:28Z

index-test.js

+    program
+      .option('--save-sequence [string]', 'Name space separated with Stringified sequence');
+
+    program.parse(['node', 'test', '--save-sequence']);


Oh cool, is this the key function, .parse()? via Commander?

This looks great. Could we rename the file something like cli-test.js or index-cli-test.js so it's clear? And, perhspas should we place it in the test directory?

Thanks so much! This is super awesome! And very readable!

Oh cool, is this the key function, .parse()? via Commander?

Yup we pass can pass the user arguments through it. More info here

This looks great. Could we rename the file something like cli-test.js or index-cli-test.js so it's clear? And, perhspas should we place it in the test directory?

I named it index-test since currently it's an standalone executable command for our program.
So the convention is ./index.js <name of command> tries to find index-<name of command> and execute it. More info here

I am still not familiar with the how exactly the current tests are running, I will try to move it to our test directory once I get more familiar with it since it doesnt have a dependency on index.ts as such so I believe that's possible.

Thanks so much! This is super awesome! And very readable!

Thanks 😄

ahhh, i see. Yes, let's try to put it into test, but, if the convention doesn't accommodate subdirectories, we can just put a really clear explanatory comment at the top of the file. How does that sound? Thanks!!

jywarren · 2020-10-06T16:28:27Z

Thank you! Would you be interested in being a feature-maintainer for the CLI suite? I believe it would be a pretty low commitment, as the CLI suite doesn't change much; all we're doing is building out our tests for CLI functionality.

You can see a bit more on this here: #1633

daemon1024 · 2020-10-06T18:00:30Z

@jywarren Sure. From what I understand I would need to keep it undisrupted as we progress with more features/modules once the suite has been somewhat made. The current focus would be building the suite.

jywarren · 2020-10-14T15:47:08Z

Hmm, test-ui-2 errored with:

● Cannot log after tests are done. Did you forget to wait for something async in your test? Attempted to log "TAP version 13".

The benchmark tests failed with an Electron installation issue. I restarted them to see if it's intermittent, like from an upstream download of electron? 🤞

jywarren · 2020-10-14T23:49:38Z

Hmm, i'm seeing some odd messages here in Jest tests:

● Cannot log after tests are done. Did you forget to wait for something async in your test?

jywarren · 2020-10-14T23:49:47Z

Restarting to see what happens!

daemon1024 · 2020-10-15T16:32:06Z

@jywarren I will look into it. I just tested it out by running the test file individually. Need to check how it behaves with jest.

jywarren · 2020-10-16T21:09:32Z

Awesome, thank you!

daemon1024 · 2020-10-17T16:14:57Z

@jywarren It seems that the test is failing because it's running part of the ui-tests . Need to create a new suite command to run it or I can include it under core tests. What do you think? Also are the tests documented somewhere, might help me progress faster.

jywarren · 2020-10-17T19:03:13Z

Hi! We should have better test documentation, you're right. For now you can try separating things better in .travis.yml following perhaps docs in https://docs.travis-ci.com/user/build-stages or https://docs.travis-ci.com/user/speeding-up-the-build/ ?

Thank you!!!

jywarren · 2020-10-22T13:46:32Z

Oh awesome, you got tests passing! I'm updating the branch, let's see!

harshkhandeparkar · 2020-10-22T13:57:16Z

Umm, will this be merged before v3.6.0?

jywarren · 2020-10-22T13:58:21Z

I wasn't going to, what do you think?

…

On Thu, Oct 22, 2020, 9:57 AM Harsh Khandeparkar ***@***.***> wrote: Umm, will this be merged before v3.6.0? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1718 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J4DOOZZW3GDMV77ODLSMA25DANCNFSM4SGFG62Q> .

jywarren · 2020-10-22T13:59:24Z

Shall we make a project for 3.6.1? Or is this test suite an extra 3.7.0?

…

On Thu, Oct 22, 2020, 9:58 AM Jeffrey Yoo Warren ***@***.***> wrote: I wasn't going to, what do you think? On Thu, Oct 22, 2020, 9:57 AM Harsh Khandeparkar ***@***.***> wrote: > Umm, will this be merged before v3.6.0? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1718 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAAF6J4DOOZZW3GDMV77ODLSMA25DANCNFSM4SGFG62Q> > . >

test/cli/saveSequence.js

harshkhandeparkar · 2020-10-22T14:04:29Z

Shall we make a project for 3.6.1? Or is this test suite an extra 3.7.0?
…
On Thu, Oct 22, 2020, 9:58 AM Jeffrey Yoo Warren @.> wrote: I wasn't going to, what do you think? On Thu, Oct 22, 2020, 9:57 AM Harsh Khandeparkar @.> wrote: > Umm, will this be merged before v3.6.0? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1718 (comment)>, > or unsubscribe > https://github.com/notifications/unsubscribe-auth/AAAF6J4DOOZZW3GDMV77ODLSMA25DANCNFSM4SGFG62Q > . >

I have already added this to the 3.6.1 project. 3.6.0 is ready, can you merge the last PR? Then we can merge this one...

jywarren · 2020-10-22T14:16:15Z

Great, merged that one. In the checklist in #1692 i have a few steps before we can resume merging to main, i believe! But once we merge to stable OR create a release (not sure which) we can merge this. Thanks, all!

test/cli/saveSequence.js

welcome · 2020-10-29T11:42:12Z

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will be published to https://beta.sequencer.publiclab.org in a day or two. Please test out your work on this testing server and report back with a comment that all has gone well!
In the meantime, can you tell us your Twitter handle so we can thank you properly also do join our weekly check-in to share your this week goal and the awesome work you did 😃.
Please find the link pinned in the issue section
Now that you've completed this, you can help someone else take their first step! Try looking at this list of first-timers-only issues, and see if someone else is waiting for feedback, or even stuck! 😕
People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉
See: Public Lab's coding community!

jywarren · 2020-10-29T11:42:51Z

Awesome!!!!!

test: add a basic cli test for savesequence

7bccdef

jywarren reviewed Oct 6, 2020

View reviewed changes

Merge branch 'main' into cli-test

e6bd276

refactor cli-test

f2a9980

jywarren mentioned this pull request Oct 14, 2020

Create CODEOWNERS #1719

Merged

5 tasks

Merge branch 'main' into cli-test

4dfd28e

Merge branch 'main' into cli-test

4a75ce2

jywarren requested a review from a team as a code owner October 14, 2020 23:49

jywarren mentioned this pull request Oct 16, 2020

Fixes error on | character in CLI #1634

Draft

5 tasks

Merge branch 'main' into cli-test

e191fc2

Run cli tests seperately

6148a25

daemon1024 requested a review from a team as a code owner October 22, 2020 07:41

Merge branch 'main' into cli-test

6b69663

harshkhandeparkar reviewed Oct 22, 2020

View reviewed changes

test/cli/saveSequence.js Show resolved Hide resolved

harshkhandeparkar reviewed Oct 22, 2020

View reviewed changes

test/cli/saveSequence.js Show resolved Hide resolved

Merge branch 'main' into cli-test

1005760

jywarren approved these changes Oct 22, 2020

View reviewed changes

harshkhandeparkar reviewed Oct 22, 2020

View reviewed changes

test/cli/saveSequence.js Show resolved Hide resolved

jywarren mentioned this pull request Oct 28, 2020

Bump commander from 4.1.0 to 5.1.0 #1664

Closed

jywarren added this to the v3.7.0 milestone Oct 28, 2020

harshkhandeparkar approved these changes Oct 29, 2020

View reviewed changes

Merge branch 'main' into cli-test

f336083

jywarren merged commit a5e3584 into publiclab:main Oct 29, 2020

daemon1024 mentioned this pull request Oct 30, 2020

CLI Test Suite #1747

Open

7 tasks

jywarren mentioned this pull request Nov 1, 2020

CLI error did not trigger a Travis failure #659

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding tests for CLI functionality #1718

adding tests for CLI functionality #1718

daemon1024 commented Oct 6, 2020 •

edited

Loading

gitpod-io bot commented Oct 6, 2020 •

edited

Loading

codecov bot commented Oct 6, 2020 •

edited

Loading

daemon1024 commented Oct 6, 2020

jywarren Oct 6, 2020

daemon1024 Oct 6, 2020 •

edited

Loading

jywarren Oct 6, 2020

jywarren commented Oct 6, 2020

daemon1024 commented Oct 6, 2020

jywarren commented Oct 14, 2020

jywarren commented Oct 14, 2020

jywarren commented Oct 14, 2020

daemon1024 commented Oct 15, 2020

jywarren commented Oct 16, 2020

daemon1024 commented Oct 17, 2020

jywarren commented Oct 17, 2020

jywarren commented Oct 22, 2020

harshkhandeparkar commented Oct 22, 2020

jywarren commented Oct 22, 2020 via email

jywarren commented Oct 22, 2020 via email

harshkhandeparkar commented Oct 22, 2020

jywarren commented Oct 22, 2020

welcome bot commented Oct 29, 2020

jywarren commented Oct 29, 2020

adding tests for CLI functionality #1718

adding tests for CLI functionality #1718

Conversation

daemon1024 commented Oct 6, 2020 • edited Loading

gitpod-io bot commented Oct 6, 2020 • edited Loading

codecov bot commented Oct 6, 2020 • edited Loading

Codecov Report

daemon1024 commented Oct 6, 2020

jywarren Oct 6, 2020

Choose a reason for hiding this comment

daemon1024 Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

jywarren Oct 6, 2020

Choose a reason for hiding this comment

jywarren commented Oct 6, 2020

daemon1024 commented Oct 6, 2020

jywarren commented Oct 14, 2020

jywarren commented Oct 14, 2020

jywarren commented Oct 14, 2020

daemon1024 commented Oct 15, 2020

jywarren commented Oct 16, 2020

daemon1024 commented Oct 17, 2020

jywarren commented Oct 17, 2020

jywarren commented Oct 22, 2020

harshkhandeparkar commented Oct 22, 2020

jywarren commented Oct 22, 2020 via email

jywarren commented Oct 22, 2020 via email

harshkhandeparkar commented Oct 22, 2020

jywarren commented Oct 22, 2020

welcome bot commented Oct 29, 2020

jywarren commented Oct 29, 2020

daemon1024 commented Oct 6, 2020 •

edited

Loading

gitpod-io bot commented Oct 6, 2020 •

edited

Loading

codecov bot commented Oct 6, 2020 •

edited

Loading

daemon1024 Oct 6, 2020 •

edited

Loading