ponylang / pony-stable

:horse: A simple dependency manager for the Pony language.
BSD 2-Clause "Simplified" License
134 stars 18 forks source link

simple bundle.json validation #66

Closed srenatus closed 6 years ago

srenatus commented 6 years ago

This is quite minimal, but it ~should fix~ is a start for #48.

Now, both

{}

and

{
  "type": "github",
  "repo": "oraoto/pony-websocket"
}

result in errors, while the empty bundle still is accepted:

{"deps":[]}

I've tried to keep duplication low, but I'm also still new to Pony. Happy to take directions :)

srenatus commented 6 years ago

One oddity is that using this bundle.json,

{
  "deps": [
    { "tyxpe": "github", "repo": "jemc/pony-jason" }
  ]
}

the output is

$ ../pony-stable/build/release/stable fetch
failed to parse dependency: {"tyxpe":"github","repo":"jemc/pony-jason"}
failed to parse dependency: {"tyxpe":"github","repo":"jemc/pony-jason"}

-- So it seems Bundle().deps() is created twice? Or I botched something up.

Another data point, using an empty object bundle.json, {}, I get this:

JSON error at: /Users/stephan/Misc/pony/pony-crdt/bundle.json: missing "deps" array
JSON error at: /Users/stephan/Misc/pony/pony-crdt/bundle.json: missing "deps" array
No bundle.json in current working directory or ancestors.

I suspect something is wonky in this section: https://github.com/ponylang/pony-stable/blob/353733f5933bab2fe51e7938abcd8b0267119e14/stable/main.pony#L38-L50

SeanTAllen commented 6 years ago

@srenatus Seems like this would be a good time to start a test suite that covers some basic cases. I'm not comfortable merging this change without some assurance it is working. A basic automated suite would be good.

I don't have much time but I could assist as best I can with that.

srenatus commented 6 years ago

@SeanTAllen agreed! I'd feel better with tests here, too. :) I'll try adding some tests to this PR this week. (Doing this one the side, so please bear with me if it takes a bit longer.)

srenatus commented 6 years ago

Ok, I've updated the branch: there's now some tests for Bundle(...), both reading existing file, and a simple check for when it's supposed to create an empty file itself.

Looks like circleci was instructed to run make test anyways, so this is already wired up: https://circleci.com/gh/ponylang/pony-stable/66

I've also reverted a but of what I've proposed initially: this is now only checking for the deps key, the values themselves aren't checked yet.

@SeanTAllen How does that look? I'd be grateful for any suggestions :)

SeanTAllen commented 6 years ago

I think @jemc is the best person to evaluate this as he has experience on how tests should be set up for applications and I have zero real experience with that.

srenatus commented 6 years ago

@SeanTAllen, @jemc thank you both! 😃

jemc commented 6 years ago

Thank you!