Skip to content
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

Add list of specs working on win32 in std_spec #8664

Merged

Conversation

straight-shoota
Copy link
Member

This collects all specs currently working on win32 (working does not neccesarily they're passing) in a single spec, so they can be easily compiled using the crystal-windows helper from the Porting to Windows guide.

The current spec stats:

Finished in 2.81 seconds
3908 examples, 13 failures, 191 errors, 8 pending

The stdlib spec suite on linux64 currently runs 9110 examples, so we're currently at about half.
Most of the currently disabled specs are probably working as well. I've simply disabled entire files or even directories, but often there are just a few specs that don't work on win32 but exclude the entire file. This can be improved obviously by more fine-grained control.

The 5% failing specs are all in path and dir spec.

This list gives an overview over which specs are currently working on win32 and what not.
There are different levels of not-working:

  • Specs that error at compile time are completely commented out. This is mostly caused by some API not being ported to win32 (either the spec target or the spec uses non-ported features).
  • Specs that compile but don't link (at least not on basis of the Porting to Windows guide) are guarded by the require_nolink macro. When passing -Dwin32_nolink they can be enabled for testing codegen. Most of are just caused by missing libraries (libxml2, libyaml, libgmp, libllvm, libz, libssl), but there also seem to be some incompatibilities with the existing libraries.
  • Some of the require_nolink specs are annotated as # execution breaks, which means linking is fine but they break the entire spec suite at runtime. I assume it's related to the very basic exception handling on win32.

/cc @oprypin

@straight-shoota straight-shoota added platform:windows Windows support based on the MSVC toolchain / Win32 API kind:specs labels Jan 8, 2020
@RX14
Copy link
Contributor

RX14 commented Jan 9, 2020

See also RX14@990f8bf for an alternative way of doing this.

Your method is neater, but can it be automated to update the list of what works? At this stage of development, there's a lot of work in the corelib which ends up making a lot of specs work at the same time.

@straight-shoota
Copy link
Member Author

Oh, nice. I didn't think about automating this.

Your method is neater, but can it be automated to update the list of what works?

This could probably work in a similar way as your script. The main difference is that my variant combines directory contents in a wildcard if all files run. That makes it neater. We could probably implement this in the script, but it's probably not really worth it. I don't mind having a long list of all files.

So I'd prefer your solution since it's easier to maintain. We should better invest our efforts in making things work on windows.

@RX14
Copy link
Contributor

RX14 commented Jan 9, 2020

@straight-shoota will you update this PR with something similar to my method then or?

@straight-shoota
Copy link
Member Author

Sure, can do that.

You should definitely share more of your goodies that have been laying around for 2 years!! 😆

@RX14
Copy link
Contributor

RX14 commented Jan 9, 2020

@straight-shoota just check my windows branch!

All it has is the Process refactor which is very similar to #8518, this specs commit, and "shitty WIP stubbing" in the compiler (which gets the crystal compiler to build and work on windows, this is doable right after #8518 is merged)

@straight-shoota
Copy link
Member Author

I've improved the generator to use multiple stages in order to avoid compiling the same spec file twice and to distinguish codegen and linking failures.

New stats:

Finished in 6.13 seconds
4917 examples, 14 failures, 192 errors, 8 pending

P.S. We should consider printing the spec results at the very end when the listing of failed examples already fills 8 pages...

@straight-shoota
Copy link
Member Author

With a single character fix (#8667), we're down to 6 errors 🎉

Finished in 6.21 seconds
4917 examples, 14 failures, 6 errors, 8 pending

So I guess, never mind the suggestion for improving spec results 😄

Copy link
Contributor

@RX14 RX14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!!!!!!!!!

spec/spec_helper.cr Outdated Show resolved Hide resolved
@yxhuvud
Copy link
Contributor

yxhuvud commented Jan 10, 2020

So I guess, never mind the suggestion for improving spec results smile

You know, I've actually wanted to have the stats line last a bunch of times when working with rspec too. It doesn't matter when there is few issues, but with many (think rails version upgrades), things get problematic fast.

@RX14
Copy link
Contributor

RX14 commented Jan 10, 2020

feel free to merge @straight-shoota

@straight-shoota straight-shoota merged commit 912c4d4 into crystal-lang:master Jan 10, 2020
@straight-shoota straight-shoota deleted the feature/win32-spec branch January 10, 2020 13:56
@bcardiff bcardiff added this to the 0.33.0 milestone Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:specs platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants