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 test for invalid task metadata #120

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

timidri
Copy link

@timidri timidri commented Sep 21, 2021

Add test for task metadata file with invalid escape character.
This test is expected to fail on invalid json, but instead it fails on invalid schema - the task metadata files have a different schema. I'm not sure what exactly happens in pdk validate when no error is raised on this task metadata json.

@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #120 (7aa6617) into master (6919241) will not change coverage.
The diff coverage is n/a.

❗ Current head 7aa6617 differs from pull request most recent head f0ebdcd. Consider uploading reports for the commit f0ebdcd to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #120   +/-   ##
=======================================
  Coverage   50.23%   50.23%           
=======================================
  Files           5        5           
  Lines         211      211           
=======================================
  Hits          106      106           
  Misses        105      105           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6919241...f0ebdcd. Read the comment docs.

@ekohl
Copy link
Member

ekohl commented Sep 21, 2021

This test is expected to fail on invalid json, but instead it fails on invalid schema - the task metadata files have a different schema. I'm not sure what exactly happens in pdk validate when no error is raised on this task metadata json.

I suspect that there is no check for this so it falls back to the regular metadata checks. I'd start with adding all missing fields.

@timidri
Copy link
Author

timidri commented Sep 21, 2021

This test is expected to fail on invalid json, but instead it fails on invalid schema - the task metadata files have a different schema. I'm not sure what exactly happens in pdk validate when no error is raised on this task metadata json.

I suspect that there is no check for this so it falls back to the regular metadata checks. I'd start with adding all missing fields.

Sure, I can do that, but do we want metadata-json-lint to also correctly validate task metadata files? Or should that be a separate new feature request?

@ekohl
Copy link
Member

ekohl commented Sep 21, 2021

AFAIK this gem has never done anything with task metadata files. Given all technologies are in here to make it happen, I can see it being in scope for this gem but it would be a new feature request.

@timidri
Copy link
Author

timidri commented Oct 11, 2021

AFAIK this gem has never done anything with task metadata files. Given all technologies are in here to make it happen, I can see it being in scope for this gem but it would be a new feature request.

That makes sense. For now, I've changed the test to use a module metadata file which shows the same problem.

@ekohl
Copy link
Member

ekohl commented Nov 2, 2021

This looks like a valid reproducer. Now all we need is someone to write a patch to make the tests pass.

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