xdpm
xdpm copied to clipboard
CI for xdpm with GitHub actions
Description
Added a GitHub actions workflow to add some basic CI, meaning Code Review etc. no longer needs everyone to pull the current changes, but just requires looking at the output of the CI.
Currently, I haven't found a way to let the test fail when something that should fail passes (e.g., if xdpm validate
passed while outside a plugin dir), but it is a first step into the right direction; I'm more than open for input on how better CI could get achieved for such a CLI.
Related Issue
This somewhat relates to current other PRs where I've been doing QA. Always having to clone the project to check output is kind of annoying and slows down the review process by a lot, but this doesn't necessarily fix any particular issues.
Motivation and Context
How Has This Been Tested?
This is a test by itself. Testing tests would get kind of Meta (who would test this next level of tests, after all), so I've manually tried it :stuck_out_tongue_winking_eye: . It is, however, not really functional, meaning it only is required to be a valid GH Actions workflow...
Screenshots (if appropriate):
n/a
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] Other (Meta)
Checklist:
- [x] I have signed the Adobe Open Source CLA.
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation (maybe?).
- [ ] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [x] I have added tests to cover my changes (?).
- [x] All new and existing tests passed.
Thanks Pablo. Trying to have a look. We aren't quite sure how to test/debug an action before merging, so looking into that first.
Actions are new to me so I'm curious: how were you able to run the action while developing it?
Thanks Pablo. Trying to have a look. We aren't quite sure how to test/debug an action before merging, so looking into that first.
Actions are new to me so I'm curious: how were you able to run the action while developing it?
Of course. You can see the (current) output of it here. It really is committing, looking at the output and iterating. In this case, it wasn't too complicated, as it's basically just running a sequence of commands, but otherwise, I really just commit, wait for the CI logs, look at it, try to correct mistakes and so on over and over again (resulting in very boring commit messages such as Trying to get CI working, Fixing the CI config (probably?) and similar :wink:).
Quite honestly, I'm already kind of familiar with Actions (as I had joined the Beta and used them in some server-state-repos), meaning this was done in relatively few commits (and it was basically just adding things I knew how to do) and when I initially tested those first Actions, it really was just as stated above: countless commits leading up to a working CI, so -- there's that :slightly_smiling_face:.
I have to say, however: Since GH Actions run quite fast, this is ok as a workflow. I remember building something for GitLab CI and it took me ~15 hours or so to get it working (every change required around 12 minutes of execution in the CI until I got results based on which I could iterate...)
OK! I spent a little time getting up to speed on actions and these are really cool. Thanks for start this.
Mind if I contribute some work to the branch on your fork?
I haven't found a way to let the test fail when something that should fail passes (e.g., if xdpm validate passed while outside a plugin dir)
Actually, @kerrishotts and I were just looking at this and we think your test and the actions are working as expected. Instead, your test has exposed a bug in xdpm.
From one of your runs:
Run xdpm validate 0s
23
##[error]Process completed with exit code 1.
1
Run xdpm validate
4
INFO: xdpm 3.1.0 - XD Plugin Manager CLI
5
INFO: Use of this tool means you agree to the Adobe Terms of Use at
6
https://www.adobe.com/legal/terms.html and the Developer Additional
7
Terms at https://wwwimages2.adobe.com/content/dam/acom/en/legal/servicetou/Adobe-Developer-Additional-Terms_en_US_20180605_2200.pdf.
8
/home/runner/work/xdpm/xdpm/lib/validate.js:37
9
manifest.icons.forEach((icon, idx) => {
10
^
11
12
TypeError: Cannot read property 'forEach' of undefined
13
at validate (/home/runner/work/xdpm/xdpm/lib/validate.js:37:20)
14
at /home/runner/work/xdpm/xdpm/commands/validate.js:43:24
15
at Array.map (<anonymous>)
16
at validatePlugin (/home/runner/work/xdpm/xdpm/commands/validate.js:30:26)
17
at Object.<anonymous> (/home/runner/work/xdpm/xdpm/index.js:72:39)
18
at Module._compile (internal/modules/cjs/loader.js:959:30)
19
at Object.Module._extensions..js (internal/modules/cjs/loader.js:995:10)
20
at Module.load (internal/modules/cjs/loader.js:815:32)
21
at Function.Module._load (internal/modules/cjs/loader.js:727:14)
22
at Function.Module.runMain (internal/modules/cjs/loader.js:1047:10)
23
##[error]Process completed with exit code 1.
In manifest.icons.forEach
, icons
is undefined so the code is failing.
I'm not yet sure if fixing this in the code will mean that since the script fails in the right way the test will pass, but we'll need to fix the bug either way.
@ashryanbeats Yes, that's a bug with xdpm
, but the problem is that these steps are currently configured as continue-on-error=true
, meaning CI reports success even though these tests (expectedly) fail. They should, however, fail, as I've added tests for invalid things to test this aspect of xdpm
as well. If this, however, were to pass (which, of course, it shouldn't), the workflow would (wrongly) succeed (while it should actually fail as xdpm
had an exit code of 0
where it shouldn't have succeeded)...
Basically, my CI workflow currently has a problem with false negatives in xdpm
's manifest validation, if there were any...
Thanks @pklaschka. I'm starting to wonder if the point of GitHub Actions is that we should be writing things that are supposed to pass, rather than checking for things that are supposed to fail (although I too want to do both).
I did a little hacking on the actions you set up: https://github.com/ashryanbeats/xdpm/blob/github-actions/.github/workflows/cli-test.yml
In that fork, the changes worth noting:
- [action] Now running jobs on Mac and Win
- [action] Ensuring
xdpm install
action doesn't fail due to non-existent target dirs by usingmkdir/md
- [lib/validate.js] Ensuring
xdpm validate
action doesn't fail by checking if.icons
property existing before invoking.forEach
Here's the result: https://github.com/ashryanbeats/xdpm/runs/346161827
You'll note that there are no more failure annotations. This may not be the final result we're looking for, but I'm hoping this is helpful.
Another thought to keep in mind is that the bootstrap
command, when available in 4.0, should be quite useful in terms of running the other xdpm commands for success scenarios.
Note: edited 2 comments up to link to the correct branch's yml file.
@ashryanbeats Oops, sorry. You'll have to open another PR into pklaschka:github-actions to get reflected here. My master branch has some experimental Linux Subsystem for Windows stuff in it, too, cf. https://github.com/AdobeXD/xdpm/pull/21. We've accidentally merged this into my master-branch, which is therefore not usable for this PR...
Curent CI results: https://github.com/pklaschka/xdpm/runs/372148630
LGTM, but I'd like to write a test coverage instead of continue-on-error=true
trick. Probably it's can be merged, to at least validate PRs in a way "it builds successfully or not?"