Get input from commandline when instance(s) not provided #643
Conversation
Codecov Report
@@ Coverage Diff @@
## master #643 +/- ##
=========================================
- Coverage 95.12% 94.4% -0.72%
=========================================
Files 18 18
Lines 2419 2486 +67
Branches 309 309
=========================================
+ Hits 2301 2347 +46
- Misses 97 118 +21
Partials 21 21 |
|
Thanks, definitely reasonable! I'd very much prefer not using mock for testing though -- there should (already I think) be a way to just pass the input you want on stdin -- can you give that a shot? If there isn't, it should be fairly easy to add, but if you need some direction happy to help there. |
|
@Julian, I made the requested change, what do you think? Not sure why the checks are failing as I didn't touch those files. |
There was a problem hiding this comment.
Thanks.
The failing tests look like they're GitHub pages rejecting scripting requests, which must have changed yesterday. I've pushed a fix in 5f296fd so you should be able to pull that commit in.
Left another 2 small comments, but think we're getting close.
Appreciated!
jsonschema/tests/test_cli.py
Outdated
| @@ -3,6 +3,11 @@ | |||
| import subprocess | |||
| import sys | |||
|
|
|||
| try: | |||
| from StringIO import StringIO | |||
There was a problem hiding this comment.
The type of stdin.read across versions is "native string", which unfortunately the stdlib doesn't really provide a NativeStringIO class for, so there's one in jsonschema.compat called NativeIO (which is already used in lots of tests -- it's the same thing you need here for stdin).
jsonschema/tests/test_cli.py
Outdated
| @@ -149,3 +154,20 @@ def test_version(self): | |||
| ) | |||
| version = version.decode("utf-8").strip() | |||
| self.assertEqual(version, __version__) | |||
|
|
|||
| def test_piping(self): | |||
| sys.stdin = StringIO("{}") | |||
There was a problem hiding this comment.
Thanks, this definitely now doesn't use mock, but it does now leak global state across test runs -- from here on out, every test will see sys.stdin as being the StringIO here.
What I had in mind was more that you should pass the input into the cli.run call below.
Specifically, if we add a stdin argument to run (with a default of sys.stdin), then you can pass your NativeIO for stdin to run, same as how stdout and stderr are being passed to it.
talham7391
force-pushed
the
enable-cli-piping
branch
from
069db1c to
8ce3302
Compare
|
Looks like the Code Cov is missing repository upload token. |
|
The coverage thing is https://github.community/t5/GitHub-Actions/Make-secrets-available-to-builds-of-forks/td-p/30678/highlight/true/page/2 which I haven't fully made my mind up on whether to disable it for PRs or just make the token public. Otherwise think everything looks good to go -- thanks! Going to have one last look and then merge. |
19947eaa1 test: unevaluatedProperties not affected by propertyNames 829270631 Check that large integers are multiples of small multipleOf b59543f6e Merge pull request #647 from santhosh-tekuri/ref-start-slash 6e5d45d71 Merge pull request #646 from santhosh-tekuri/vocab-optional 0311dfda0 Merge pull request #651 from santhosh-tekuri/dynamicref-without-anchor 4503eeaf4 test: A $dynamicRef without anchor in fragment behaves identical to $ref 39af4c1ba test: $ref with absolute-path-reference 880c9933b test/vocabulary: ignore unrecognized optional vocabulary fd80307ff Merge pull request #642 from santhosh-tekuri/time-2digit a76ae650d Merge pull request #645 from json-schema-org/gregsdennis/add-vocab-tests-link 0e2b4eefd Merge pull request #643 from 0xSudarshan/main 2b78ccfc4 slight tweaking to wording 8716c4054 add link for vocab test suite to readme c49ba5445 Fix an incorrect $schema identifier. f0e5ce71e Added test for schema-items alongside "ignored" additionalItems 76dae88ab Merge pull request #640 from santhosh-tekuri/refRemote-anchor cb82e237c Merge pull request #641 from json-schema-org/gregsdennis/unevaluated-not-draft-next e4afd233a test/format: hour, min, sec must be two digits 7efd51313 fix unevaluatedProperties/not tests for draft-next e39c6ea6a test/refRemote: anchor within remote ref bf51b32fb Merge pull request #639 from json-schema-org/additionalItems-unevaluatedItems 52160b368 Add a test for 2019's interaction between additional/unevaluatedItems 69a09a339 Fixed tests for annotation collection inside not. e4e1a220b Draft7 if/then/else ref tests need to be wrapped in an allOf. f5bd2f6c3 Merge pull request #632 from json-schema-org/ether/annotations-inside-not 626b433e5 test that annations are collected inside a "not" git-subtree-dir: json git-subtree-split: 19947eaa1289168a49edd21bb7a8aa2098069ae0
Added for this: #621
Can now do something like this:
cat test.json | jsonschema test.schema