search

ThinkOpenly / sail

Sail architecture definition language
Other
11 stars 12 forks source link

Ci for json validation #31

Closed snapdgn closed 5 months ago

snapdgn commented 5 months ago

Potential Fix: #30

github-actions[bot] commented 5 months ago

Test Results

    9 files  ±0     20 suites  ±0   0s :stopwatch: ±0s   615 tests ±0    427 :white_check_mark:  - 188  0 :zzz: ±0  0 :x: ±0  188 :fire: +188  1 978 runs  ±0  1 743 :white_check_mark:  - 234  1 :zzz: ±0  0 :x: ±0  234 :fire: +234 

For more details on these errors, see this check.

Results for commit 3bfada0e. ± Comparison against base commit 7ad136c5.

:recycle: This comment has been updated with latest results.

snapdgn commented 5 months ago

@ThinkOpenly, this should be done now. Initially I was working on writing a different workflow(file) for this with two different approaches. One involved building/installing sail again and then validating, which was adding up to the cost of resources, and another transferring build artifacts over from build.yml to the new validate-json, and there was this whole issue of setting up ocaml/sail properly.

So I went with the simplest approach and added a new run in the same build workflow, which runs just after the install step. I've tested it against my own repo and it works.

Note:

ThinkOpenly commented 5 months ago

https://github.com/ThinkOpenly/sail-riscv/pull/28 is now merged. I've restarted the failed jobs here. :crossed_fingers:

ThinkOpenly commented 5 months ago
  • I removed the (etc/ci_core_tests.sh) from sail, since we don't rely on testing now. Let me know if i should add this.

How strongly do you feel about this? Previous runs have been on the order of a couple of minutes. I'm inclined to leave it in.

snapdgn commented 5 months ago
  • I removed the (etc/ci_core_tests.sh) from sail, since we don't rely on testing now. Let me know if i should add this.

How strongly do you feel about this? Previous runs have been on the order of a couple of minutes. I'm inclined to leave it in.

Honestly not much, it adds maybe couple seconds to a minute. I believe we can afford it. I've added the tests back.

P.S: ~ 2 minutes image