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

fix: JSDoc null values being interpreted as strings, and added test README #1377

Merged
merged 3 commits into from Sep 5, 2022

Conversation

adam-coster
Copy link
Contributor

@adam-coster adam-coster commented Aug 26, 2022

fixes #1373

  • Includes a test case for default values of each JSON-compatible type.
  • Adds an extendible option argument to the schema-validator testing function, with current options allowing passing valid/invalid samples to confirm that the schema behaves as expected
  • Adds a very brief README to the tests/ folder, outlining the basic steps of setting up the kind of test required for this PR (I had to do a lot of project exploration to figure out how to go about setting up a functional test case).

@adam-coster
Copy link
Contributor Author

@domoritz I'm not sure what to do about the single failing test.

The tests for the changes I made for the associated issue ran successfully.

The failing test is due to a mismatch in the expected vega-lite schema. When I ran tests locally there was a failure related to that same schema, which passed when I ran the tests with the UPDATE option. I included that updated schema with this PR and made sure that the test script passed all tests locally.

@domoritz
Copy link
Member

Nuke your node modules maybe?

@adam-coster
Copy link
Contributor Author

@domoritz I've updated the PR to only include the commits that change the files related to the reported issue. Tests should now pass.

It looks like vega-lite was bumped to 5.5.0 right around when I originally forked the project, so the prior issues must have been caused by something going awry when I merged from upstream after that.

test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
@adam-coster
Copy link
Contributor Author

@domoritz I've added a commit with the requested changes to the test/README.md file

@domoritz domoritz changed the title Fixed JSDoc null values being interpreted as strings, and added test README fix: JSDoc null values being interpreted as strings, and added test README Sep 5, 2022
@domoritz domoritz enabled auto-merge (squash) September 5, 2022 01:46
@domoritz domoritz merged commit 277b89a into vega:next Sep 5, 2022
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.

JSDoc null annotations are treated as strings
2 participants