Add assert-cli tests for wasm-pack #61
Conversation
|
fyi this looks like it needs at least a rustfmt (nightly) run, also fails on windows? (also yay thank uu) |
|
The progress bar might only show if the ouput is a tty, or to write to stderr. |
|
Got it. It didn't show up in either so that is unfortunate. Yeah I'll definitely run a rustfmt |
|
I ran rustfmt and then changed the commits to fit your prefered format like you had with my other PR @ashleygwilliams. I think this should be good to go. |
|
fwiw i'm not super strict about the commit message stuff but figured i'd edit cuz i was adding a commit and doing a fixup anyways <3 (i do appreciate you following it tho 😄) |
|
hrm, this is still failing on appveyor and i'm not familiar enough with assert-cli to know what's causing the issue. @killercup or @mgattozzi any thoughts? |
|
Oh I think I know why. It timed out but appveyor probably didn't have wasm-bindgen installed and cached so it took too long. |
|
@mgattozzi yeah that was my initial thought too, don't know if we can pass a higher timeout to appveyor? |
|
I think the simpler solution might be to have appveyor build wasm-bindgen as part of the build script. Part of the timeout seemed to be cargo and not necessarily AppVeyor looking at the logs again. |
|
true. happy to accept an update to the appveyor config that preloads it. can probably just add it as a step here https://github.com/ashleygwilliams/wasm-pack/blob/master/.appveyor.yml#L1 |
|
alternatively should it be a dep of this crate? i'm.. not sure. open to thoughts |
|
I just pushed the CI change for now. I think maybe we should consider adding it as a dep as part of #51 and update accordingly. |
|
ah woops it's wasm-bindgen-cli not wasm-bindgen |
We needed to add wasm-bindgen to the buildscript because assert-cli tests were timing out trying to install it when building the code to test output.
|
lol i ran into that same issue a few days ago lol. when CI passes i'll merge this |
|
Yaaaay! :D |
|
Okay so I think there's some kind of bug with assert-cli and how it tests windows. |
|
@ashleygwilliams I'm gonna close this PR for now. We can figure out this issue a bit later. I'll have to test this on my windows computer at some point and come up with a better solution. |
This works but for some reason I can't test the
[1/7][2/7]etc. because it's not showing up in the testing framework as output that happened but it does show up in the console. Not sure if this is an identicaf bug or assert-cli@killercup thoughts?
Closes #18