image-sequencer icon indicating copy to clipboard operation
image-sequencer copied to clipboard

CLI Test Suite

Open daemon1024 opened this issue 5 years ago • 14 comments

Follow up meta issue for #1694 and #1718

Aim of this suite

Write tests for various components of the CLI aspect of Image Sequencer.

Where to add the tests

test/cli/*

How to run these tests

npm run test-cli

List of needed tests

  • [ ] various kinds of parsing
  • [ ] installModule
  • [x] saveSequence
  • [ ] sequencerSteps
  • [ ] config and steps ( ref #1634 )
  • [ ] input/output
  • [ ] basic mode

Mention more in the comments if I missed out on something

How to implement

https://github.com/publiclab/image-sequencer/blob/main/test/cli/saveSequence.js has an initial implementation.

  • Use tape to setup a test.
  • Pass in your required parameters inside cli function. https://github.com/publiclab/image-sequencer/blob/0580218c6310486be7ee4275ab85fc24f85bd7eb/test/cli/saveSequence.js#L16

@publiclab/is-maintainers @publiclab/is-cli-maintainers @publiclab/is-tests-maintainers

daemon1024 avatar Oct 30 '20 20:10 daemon1024

This is great!!

Let's tie it to the documentation for the cli, like, reading the readme ought to link to corresponding tests demonstrating those features, what do you think?

We might also reuse test scenarios (input and expected output images) from other test suites like the basic module tests.

Thanks so much - this is really helpful!!

On Fri, Oct 30, 2020, 4:51 PM Barun Acharya [email protected] wrote:

Follow up meta issue for #1694 https://github.com/publiclab/image-sequencer/issues/1694 and #1718 https://github.com/publiclab/image-sequencer/pull/1718 Aim of this suite

Write tests for various components of the CLI aspect of Image Sequencer. Where to add the tests

test/cli/*

How to run these tests

npm run test-cli

List of needed tests

Mention more in the comments if I missed out on something How to implement

https://github.com/publiclab/image-sequencer/blob/main/index.js has an initial implementation.

  • create a new commander instance
  • use parse to pass arguments as user.
  • use tape for other aspects of a test.

@publiclab/is-maintainers https://github.com/orgs/publiclab/teams/is-maintainers @publiclab/is-cli-maintainers https://github.com/orgs/publiclab/teams/is-cli-maintainers @publiclab/is-tests-maintainers https://github.com/orgs/publiclab/teams/is-tests-maintainers

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/image-sequencer/issues/1747, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J4JBTFJLRZ63WYRVSDSNMRLVANCNFSM4TFNGOJA .

jywarren avatar Oct 31 '20 14:10 jywarren

Should we add something to CONTRIBUTING.md too?

harshkhandeparkar avatar Nov 02 '20 04:11 harshkhandeparkar

oh, good idea!

On Sun, Nov 1, 2020 at 11:29 PM Harsh Khandeparkar [email protected] wrote:

Should we add something to CONTRIBUTING.md too?

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/image-sequencer/issues/1747#issuecomment-720229533, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J5AF57P5TBWCOE6DI3SNYYTLANCNFSM4TFNGOJA .

jywarren avatar Nov 02 '20 05:11 jywarren

Yeah this is something I felt too. The test aren't documented well in general so documenting the CLI tests along the way will be start :)

daemon1024 avatar Nov 02 '20 05:11 daemon1024

How should I proceed btw? 😅 Open individual issues for each test scenario? Or just directly PRs.

Also should I plan them out in a project or linking them to this issue is enough?

daemon1024 avatar Nov 02 '20 09:11 daemon1024

Do whatever is convenient :) But I suppose linking them here is a minimum(to keep track of the big picture).

harshkhandeparkar avatar Nov 02 '20 09:11 harshkhandeparkar

@daemon1024 Is it possible to create a test template so that people don't have to spawn the commander instance each time? eg: /tests/core/templates/

harshkhandeparkar avatar Nov 02 '20 13:11 harshkhandeparkar

That makes sense. Let me try to implement that.

daemon1024 avatar Nov 02 '20 14:11 daemon1024

@HarshKhandeparkar What do you think about #1785. This eliminates the need to spawn commander instance each time.

P.s. Sorry I got busy with university and couldn't work on it, Now that I have holidays I hope to make some progress on the test-suite.

daemon1024 avatar Jan 02 '21 16:01 daemon1024

We should probably merge https://github.com/publiclab/image-sequencer/pull/1786 before proceeding with the suite.

daemon1024 avatar Feb 10 '21 14:02 daemon1024

Great, i left some comments and analysis there although didn't get too far. thanks!

jywarren avatar Feb 10 '21 15:02 jywarren

Since https://github.com/publiclab/image-sequencer/pull/1820 is now merged. I guess we can proceed with is.

daemon1024 avatar Feb 27 '21 12:02 daemon1024

Updated how to implement with the refactored test method.

https://github.com/publiclab/image-sequencer/blob/main/test/cli/saveSequence.js has an initial implementation.

  • Use tape to setup a test.
  • Pass in your required parameters inside cli function. https://github.com/publiclab/image-sequencer/blob/0580218c6310486be7ee4275ab85fc24f85bd7eb/test/cli/saveSequence.js#L16

daemon1024 avatar Feb 27 '21 12:02 daemon1024

I will begin working on it and try to introduce a few fto's as well.

daemon1024 avatar Feb 27 '21 12:02 daemon1024