Add test for precompiled binary failing on Windows to CI #389
Conversation
.github/workflows/ci.yml
Outdated
| - name: Baseline is correct | ||
| run: | | ||
| $EXPECTED = " Parsing ref_slice v1.2.2 (current)`r`n Parsing ref_slice v1.2.1 (baseline)`r`n Checking ref_slice v1.2.1 -> v1.2.2 (patch change)" | ||
| # Strip ANSI escapes for colors and bold text before comparing. | ||
| $RESULT = (cat output | grep 'ref_slice v1.' | sed "s,\x1B\[[0-9;]*[a-zA-Z],,g") | Out-String | ||
| compare $RESULT $EXPECTED | ||
|
|
||
| - name: Semver break found | ||
| run: | | ||
| $EXPECTED = "--- failure function_missing: pub fn removed or renamed ---" | ||
| $RESULT = (cat output | grep failure) | ||
| compare $RESULT $EXPECTED | ||
| # Ensure the following fragment (not full line!) is in the output file: | ||
| grep ' function ref_slice::ref_slice, previously in file' output |
There was a problem hiding this comment.
I added caching for baseline rustdoc JSON files in the other jobs -- would you mind adding it here too? You'll need to amend the top EXPECTED variable since right now it effectively asserts that caching isn't used (Parsing ref_slice v1.2.1 (baseline), not (baseline, cached)), and add the new caching steps as in the other jobs.
There was a problem hiding this comment.
I added caching, but also extracted building the Windows binary to a separate job as you did with the one for Ubuntu. I think it makes it a little cleaner and we don't have to manually remove the index directory if we restore the cached binary instead of building it. What is your opinion?
One may ask why I created a separate build-binary-windows job instead of using matrix strategy for the existing one. The main reason is that building the tool on Windows appears to be much slower, and if we use matrix strategy, there is currently no way for other jobs to depend only on the ubuntu case, which makes the whole CI last longer...
There was a problem hiding this comment.
The explanation here is great, but once this PR merges, we probably won't remember why the jobs are separate. We might even merge them together into a matrix, even though you clearly point out how that isn't the right thing to do.
Consider adding this as a comment in the CI file, where it will be harder to miss?
There was a problem hiding this comment.
Good idea! Added in #407
|
Upgrading to |
|
I think it's ready to go now! :) |
This exposes the issue encountered while testing the new GitHub Action on Windows machine (see current workaround obi1kenobi/cargo-semver-checks-action@2d4767c).