-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use context
instead of nested it
s to fix breaking changes in Crystal .27.1
#202
Conversation
The CI is broken: Seems we're not the only ones with failing crystal on travis: https://travis-ci.community/t/apt-get-install-crystal-0-27-1-failed/2093 |
Seems good to me. Will merge once they get the CI image updated. I suppose also updating That said I still don't understand why they've been so adamant to not allow users to specify the version of Crystal they're using. There's only |
Fixed, according to bcardiff of the Crystal core team. |
Looks like it's still failing because of the changes to integer overflow handling. |
That's weird, the overflow behavior should be disabled unless From the PR merging the overflows:
|
Very. I haven't pulled and run it myself to see what exactly is causing it, it might just be a stack recursion overflow or something else. In any case, I would imagine it's reproducible. |
It works perfectly well for me: asciineema recording (Finally a reason to try asciinema!). The |
The CI |
Yep, it's the compiler. |
970712b
to
9f5b25d
Compare
There, now we just wait for .27.2. Shouldn't be long. |
describe
in place of nested it
s to fix breaking changes in Crystal 0.27.1context
instead of nested it
s to fix breaking changes in Crystal .27.1
As of Crystal .27.1 nested
it
's are no longer allowed (crystal-lang/crystal#7297), and it raises at runtime. The breaking change affects 6 of ourit
's, hence the specs are broken.This PR simply changes the parent
it
's todescribe
's which fixes the problem.