Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CLI tests #22

Merged
merged 4 commits into from
Nov 29, 2021
Merged

Add CLI tests #22

merged 4 commits into from
Nov 29, 2021

Conversation

l0b0
Copy link
Collaborator

@l0b0 l0b0 commented Nov 19, 2021

@m-mohr How is this approach?

Contributes towards #20.

.github/workflows/test.yaml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@l0b0 l0b0 force-pushed the add-cli-tests branch 2 times, most recently from dbddc6c to ecd5a75 Compare November 19, 2021 02:20
@m-mohr
Copy link
Collaborator

m-mohr commented Nov 19, 2021

Thanks a lot. Just want to let you know I've got this on the radar as I may not get to review it in the next 2 weeks due to traveling.

@l0b0
Copy link
Collaborator Author

l0b0 commented Nov 21, 2021

OK, I just need some feedback on the general approach before adding more tests.

shell.nix Outdated Show resolved Hide resolved
.editorconfig Outdated Show resolved Hide resolved
@m-mohr
Copy link
Collaborator

m-mohr commented Nov 27, 2021

Yes, the direction is good, thanks. ospec works for me although I've never worked with it before.

Until we have a non-CLI return mode, we'd probably also need to check the content of the CLI validation output a bit better, but not sure how that works with ospec.

Please clarify or remove the additional files that seem unrelated to the tests.

@l0b0 l0b0 force-pushed the add-cli-tests branch 2 times, most recently from c036f49 to 4a4e029 Compare November 28, 2021 21:32
@l0b0 l0b0 marked this pull request as ready for review November 28, 2021 21:32
@l0b0 l0b0 requested a review from m-mohr November 28, 2021 21:32
.github/workflows/test.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@m-mohr m-mohr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We could try to run a wider variety of OS instead of so many versions of a single OS.
Do you plan to add more tests in this PR or is it ready for now?

@l0b0
Copy link
Collaborator Author

l0b0 commented Nov 29, 2021

Do you plan to add more tests in this PR or is it ready for now?

I do want to add more tests later, but this gets us started. I'd say it's ready now, although I think you have to enable GitHub Actions on this repo to actually run them. I've not tested on Windows or Mac.

@m-mohr
Copy link
Collaborator

m-mohr commented Nov 29, 2021

I can't enable anything, maybe I just need to merge

@m-mohr m-mohr merged commit 1a57418 into stac-utils:master Nov 29, 2021
@l0b0 l0b0 deleted the add-cli-tests branch November 29, 2021 21:54
@m-mohr
Copy link
Collaborator

m-mohr commented Nov 29, 2021

Seems like Windows is failing, so still some work ahead :) I can look into the Windows issues in the next days

l0b0 referenced this pull request in linz/stac-node-validator Dec 8, 2021
* tests: Verify exit code when running without parameters

Adds a lightweight testing framework, ospec.

* feat: Add GitHub Actions pipeline with test job

Runs on all non-deprecated GitHub runners for maximum portability
guarantees.

* docs: Explain how to test and how to disable testing

* tests: Verify error message when running without parameters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants