_Follow up meta issue for #1694 and #1718_
Write tests for various components of the CLI aspect of Image Sequencer.
test/cli/*
npm run test-cli
Mention more in the comments if I missed out on something
https://github.com/publiclab/image-sequencer/blob/main/test/cli/saveSequence.js has an initial implementation.
tape to setup a test.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
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 notifications@github.com
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 suiteWrite tests for various components of the CLI aspect of Image Sequencer.
Where to add the teststest/cli/*
How to run these tests
npm run test-cli
List of needed tests
- various kinds of parsing
- installModule
- saveSequence
- sequencerSteps
- config and steps ( ref #1634
https://github.com/publiclab/image-sequencer/pull/1634 )- input/output
- basic mode
Mention more in the comments if I missed out on something
How to implementhttps://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
.
Should we add something to CONTRIBUTING.md too?
oh, good idea!
On Sun, Nov 1, 2020 at 11:29 PM Harsh Khandeparkar notifications@github.com
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
.
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 :)
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?
Do whatever is convenient :) But I suppose linking them here is a minimum(to keep track of the big picture).
@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/
That makes sense. Let me try to implement that.
@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.
We should probably merge https://github.com/publiclab/image-sequencer/pull/1786 before proceeding with the suite.
Great, i left some comments and analysis there although didn't get too far. thanks!
Since https://github.com/publiclab/image-sequencer/pull/1820 is now merged. I guess we can proceed with is.
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
tapeto setup a test.- Pass in your required parameters inside
clifunction. https://github.com/publiclab/image-sequencer/blob/0580218c6310486be7ee4275ab85fc24f85bd7eb/test/cli/saveSequence.js#L16
I will begin working on it and try to introduce a few fto's as well.
Most helpful comment
I will begin working on it and try to introduce a few
fto's as well.