Guards with multiple criteria about mspec HOT 13 OPEN

@eregon

Thanks for the response. Very helpful.

First, I'll get PR's in core for the unneeded guards, as the specs pass with MSYS2 builds (Ruby 2.4 forward), but they fail with Ruby 2.3 or earlier.

Secondly, I'm going to make some changes to ruby-loco to see what's causing the spec numbers to jump around.

And if I forget about run again...

Re --random, I can't recall if it was running with -j or not. I'll do some more testing to narrow it down to folders and/or files it happens with.

from mspec.

Hello,

It sounds good, please make a PR removing the extra guards.

Yes, you can use:

guard -> { platform_is :mingw and ruby_version_is ""..."2.4" } do
  # specs
end

So the usual guards also work without a block and return a boolean when called inside guard.

See https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#guards re "greater or equal for ruby versions".

Is there a way to output all skipped tests?

Do you mean specs not executed because of guards?
Yes, there is: mspec run -Z -O (See mspec run --help).
So for instance:

$ mspec run -Z -O language
$ ruby /home/eregon/code/mspec/bin/mspec-run -Z -O language
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-linux]
[| | ==================100%================== | 00:00:00]      0F      0E 

Finished in 0.116077 seconds

67 files, 2238 examples, 0 expectations, 0 failures, 0 errors, 0 tagged, 13 guards


1 spec omitted by guard: ruby_bug #14061, 2.4...2.6:

The return keyword at top level within a begin fires ensure block before returning while loads file

1 spec omitted by guard: ruby_bug #14407, 2.5.0...2.5.1:

The defined? keyword for a scoped constant returns nil when a constant is defined on top-level but not on the class

5 specs omitted by guard: ruby_version_is ...2.4:

The predefined global constants includes TRUE
The predefined global constants includes FALSE
The predefined global constants includes NIL
Regexps with repetition does not treat {m,n}+ as possessive
The rescue keyword fails when using 'rescue' in method arguments

3 specs omitted by guard: ruby_version_is ...2.5:

top-level constant lookup on a class searches Object successfully after searching other scopes
The defined? keyword for a scoped constant returns 'constant' when a constant is defined on top-level but not on the class
The return keyword at top level within a class is allowed

3 specs omitted by guard: ruby_version_is 2.6:

A block yielded a single Array when non-symbol keys are in a keyword arguments Hash raises an ArgumentError
Module#private_constant marked constants sends #const_missing to the original class or module
The rescue keyword raises SyntaxError when else is used without rescue and ensure

In testing, the numbers jump around a little, but some may be due to parallel testing.

I think the number of examples should not change on the same setup (platform/machine/version/etc).
It might be a bug, or just some specs generated dynamically.

Could you maybe do a diff of the -fs output? (that doesn't work with -j currently though)

BTW, things got a bit squirrely with --random...

What went wrong?
Currently that's not tested in CI so it's likely to break.
Would be good to fix, but not sure I have the time.

from mspec.

Could you file the bug with --random as a separate issue actually?

from mspec.

@eregon

Re guards, I've seen a few of these:

# Guard against the Mathn library
conflicts_with :Prime do

Could these be changed to:

guard -> { ::Math.private_method_defined? :rsqrt } do

I checked it with 2.3 & 2.4, and it's value depends on whether Mathn is loaded...

FYI, locally I've got code outputting file and example count when running multi, and I'm looking at files with intermittent example counts. Also shuffling the file list...

from mspec.

@MSP-Greg I didn't realize mathn was removed in 2.5, interesting.
Right, using Prime to know if mathn is loaded is indeed not a good guard.
Thanks for the idea, I'll switch to a small variation, defined?(Math.rsqrt).

from mspec.

@MSP-Greg Done in ruby/spec@c0cb934.
It looks like some specs in core do not pass with mspec -rmathn core, but I'm not sure it's worth fixing (maybe we should just drop mathn specs as it's removed in 2.5 and long deprecated).

from mspec.

@eregon

It looks like some specs in core do not pass with mspec -rmathn core,

Tonite I was going to do some testing with older versions...

Maybe some background might help.

A long time ago, after ruby-loco seemed stable, I went thru the test-all suite. First, I wondered if there were tests that were being skipped due to issues with old Ruby versions, and also due to the older build system used with Ruby 2.3 and earlier on Windows. Secondly, both Travis and Appveyor had test totals that jumped around.

So, after re-writing ruby-loco recently, I decided to do the same with the spec tests. First thing, I needed some data to look at. I've got a patched version of MSpec that runs multi and outputs data similar to:

[   1/3702]  B     1  core/kernel/readline_spec.rb
[   2/3702]  A     4  library/matrix/inspect_spec.rb
[   3/3702]  B     1  core/regexp/casefold_spec.rb
[   4/3702]  C   196  core/regexp/compile_spec.rb
[   5/3702]  A     1  library/observer/delete_observer_spec.rb

Rather than the pid, I just assigned a letter, and the numeric value is the total number of examples. Then, I wrote a script to run the whole suite multiple times (running 20 at present), and another script to summarize the data from the twenty runs. Presently, I'm also shuffling the file list.

So, what have I found?

1. The last run I did last night was the first where all twenty passed. Previously, most runs had one or two that had a SEGV.

2. ruby-loco seems to have intermittent issues with all the lambdas and blocks flying around. The main change that I did in my last run was replacing all the guards/conflicts statements with simple if/unless blocks. MinGW Ruby may have an issue with all the contexts being saved with all the lambdas, but I'm not sure how to repo it...

3. spec files using it_behaves_like intermittently fail, although it's normally just one run in twenty. That's detailed in the summary attachment.

4. 545 files with no examples.

5. Appveyor build times vary a great deal. I suspect their systems are a two core VM on a four core system with one drive (or worse). Most of the requires in spec are require_relative. I changed many of the requires in mpsec to require_relative in the hope that it would result in less disk io.

So, that's where I am. At present I've been working with the files in ruby/ruby, as I can get at both mspec and spec from the same repo. I haven't pushed the work to my fork, but if that would be helpful... Maybe I should move over to ruby/spec?

I've attached the output from my 'summary' script. I haven't made the output 'pretty', but I added some notes that (hopefully) clarify what the numbers are.

So, how can I help?

Thanks, Greg

Summary.txt

from mspec.

@eregon

I noticed the recent updates, and ruby-loco passed as of r64844. I just noticed issue #36, and I suspect that explains the issues I had with intermittent it_behaves code when running random/parallel.

I just redid the patch files in ruby-loco, and I've now got all the spec patches I'm using in the folder patches_spec.

I mentioned in ruby/ruby that I had a strange issue with Encoding::Converter.search_convpath. Earlier today ruby-loco silent stopped on spec tests. Checking more earlier, the issue seems to be that:

Encoding::Converter.search_convpath Encoding::ASCII_8BIT, Encoding::Emacs_Mule

when running in the spec tests generates a silent SEGV. But the following does not (added .to_s calls):

Encoding::Converter.search_convpath Encoding::ASCII_8BIT.to_s, Encoding::Emacs_Mule.to_s

The patch reverts the code I previously added that was somewhat 'out of character' for the suite...
Thanks, Greg

from mspec.

@MSP-Greg Thanks, I applied a variant of the patch in r64855.

Not sure #36 affects you, how are you running specs in parallel, with -j? That uses separate processes, so there should be no shared state involved.

from mspec.

@eregon

Sorry, I wasn't quite awake... Thanks for the patch. I briefly tried to repro it this morning. Can't create the SEGV outside of spec. Probably work on it tonite.

Thanks for all your work on spec, the example numbers are now consistent.

I've got one more patch that removes some MinGW guards. Travis is testing 2.3, but Appveyor will stick with 2.4? BTW, probably a wise choice, as Windows 2.3 builds are not current (they stopped at 2.3.3 and use older build tools).

I'll move my code over to parallel.rb, and do some more testing in the next few days.

I could set it up so -j -V gives an output as shown in the above message? Maybe allow the random option to also be used with -j? I may need to add another parallel.rb file, as I think I need to hook the loop in it...

Ever thought about adding a seed option? In other software, I've found it helpful when test runs were not consistent. The -j timing might affect duplicating a previous run though.

Thanks again, Greg

from mspec.

I've got one more patch that removes some MinGW guards.

PRs are welcome.

Maybe allow the random option to also be used with -j?

Does it not work currently? I'd be happy to accept a PR fixing it in that case.

Ever thought about adding a seed option?

Please file a new issue or PR for that :)

from mspec.

@MSP-Greg

So, what have I found?

1. The last run I did last night was the first where all twenty passed. Previously, most runs had one or two that had a SEGV.

Great :)

2. ruby-loco seems to have intermittent issues with all the lambdas and blocks flying around. The main change that I did in my last run was replacing all the guards/conflicts statements with simple if/unless blocks. MinGW Ruby may have an issue with all the contexts being saved with all the lambdas, but I'm not sure how to repo it...

That sounds like a MRI bug, it would be great if you can find a reproduction and report a bug there.

3. spec files using it_behaves_like intermittently fail, although it's normally just one run in twenty. That's detailed in the summary attachment.

Interesting, we should investiagte what is the root cause of this.

4. 545 files with no examples.

Right, that's harmless but I'd be happy to accept a PR removing those files for clarity (although some of them have examples, just exluced on Windows so those should remain obviously).

5. Appveyor build times vary a great deal. I suspect their systems are a two core VM on a four core system with one drive (or worse). Most of the requires in spec are require_relative. I changed many of the requires in mpsec to require_relative in the hope that it would result in less disk io.

I'd love a PR to mspec for that :)

So, that's where I am. At present I've been working with the files in ruby/ruby, as I can get at both mspec and spec from the same repo. I haven't pushed the work to my fork, but if that would be helpful... Maybe I should move over to ruby/spec?

It's nicer for me and other ruby/spec contributors to review larger changes at ruby/spec.
Small fixes or new specs are fine to commit directly in ruby/ruby.
For MSpec, please make PRs on ruby/mspec.

I've attached the output from my 'summary' script. I haven't made the output 'pretty', but I added some notes that (hopefully) clarify what the numbers are.
Summary.txt

Thanks!
What are "Intermittent specs"? Do they fail 1 of 20 runs, or they run no examples in 1 of 20 runs?
A log of the failures would be most useful to debug what goes wrong.

Note that I won't have much time for ruby/spec in the following weeks so I will be less responsive during October.

from mspec.

What are "Intermittent specs"? Do they fail 1 of 20 runs, or they run no examples in 1 of 20 runs?

they run no examples in 1 of 20 runs

(re files with no examples) accept a PR removing those files for clarity

I was thinking maybe something like change the file extension to .rb_ so the files are kept as markers but the files glob doesn't pick them up? One option...

I will be less responsive

Everyone has been and has a right to be. I've got to re-write the multi thing for parallel.rb...

from mspec.