search

c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
508 stars 24 forks source link

My fork has an error #165

Closed pb-cdunn closed 4 years ago

pb-cdunn commented 4 years ago

I don't even know how or why this test is running, but my fork fails on the mast branch, which is the master in your upstream repo. Any ideas?

I see. The repo has this:

.github/workflows/test.yml

The failure is for only test (ubuntu-latest, stable); the other 3 tests pass. Are you seeing that too?

c-blake commented 4 years ago

Yeah. Personally, I just run ./test.sh before I commit anything that could touch the basic functionality and think this git/CI stuff is over-rated and that is what I would recommend to you. @jiro4989 and @kaushalmodi wanted/added this stuff. I see some kind of failure whenever I git push anything. If you have some idea how to prevent the failures, I'm open-minded. When I look at those logs (like the one you sent), I only see observable stores warnings, not any real errors. The failure only happens on ubuntu-latest because it only runs there, though. If you look at the other 3 you will see there is no run of the test phase.

c-blake commented 4 years ago

Well, I guess the "error" could simply be the non-zero exit of the diff program/script. But that isn't really an "error". The change in integer hash is known and the other stuff is just warning deltas. Anyway, I don't think this is a real problem.

pb-cdunn commented 4 years ago

Ok, I'll try to ignore github CI for now. This is my first experience with that. I guess it's a good thing that they provide it. I'm surprised that forks automatically run it though.

c-blake commented 4 years ago

I am also slightly surprised forks auto-run it, but OTOH forks being basically "first class" makes it unsurprising. I wouldn't be surprised if early on in GH history forks didn't run them.

Anyway, I could probably upgrade the Nim version in test.yml from 1.2.0 to 1.3.7 to maybe get an empty diff which might make the exit status 1 condition go away for now.

c-blake commented 4 years ago

Well, I tried a bit to get the diff output down to an empty string/diff-exit 0 but failed. I did get it to a more obviously unimportant looking delta, though, of just a bunch of "...." lines. https://github.com/c-blake/cligen/actions/runs/269191877 .

Anyway, I just changed test.sh to exit 0 no matter what diff finds. This seems to silence the github emails (I guess only sent upon failure). I'll bet your fork, if you go to the Actions tab, would still run them for 8 minutes or whatnot, but if you git pull into your fork you may not have to edit your spam filter settings now.

pb-cdunn commented 4 years ago

The error was this:

<   -I=, --I1=     hashset(int8)    3,2       include 1 val in I1
---
>   -I=, --I1=     hashset(int8)    2,3       include 1 val in I1
309c326
<   --U2=          hashset(uint16)  9,8       include 1 val in U2
---
>   --U2=          hashset(uint16)  8,9       include 1 val in U2
313a331

I think that means the hash order is different on the test-machine than when you created a test-case. In other words, I think you have a test that depends on hash-order. That's my best guess.

c-blake commented 4 years ago

Your guess is correct. That is an error that can happen across varying versions of Nim (as can the warning messages one sees). The current test.yml specifies a version of Nim ("devel") which produces no delta, though. So, for CI / pull request purposes I think we are all set.

Thanks, though, and have a nice weekend!