Specs that always fail?
Charles Oliver Nutter
completely filled out, always fail. I was under the impression that
specs which can't be run under MRI unmodified and without tags are
broken specs. For example, CGI:
105 files, 117 examples, 25 expectations, 0 failures, 100 errors
Generating specs that can't be made to pass is a bad idea for two reasons:
- It generates failures even on MRI, which is supposed to be what we're
specifying. A spec that can't be made to pass on any implementation is a
broken spec, and there's no way to make these examples pass.
- It requires tagging dozens of failures, which means that as those
examples are filled out they still won't be run until tags are updated
again. This masks both new successes and new valid failures.
I strongly recommend we do not generate specs that have hard failures in
them. If we want a mechanism by which we can track specs that haven't
been implemented, there should be another counter in mspec, or an
ability to pass no block and have it be recorded as an "example-less
spec" for fill-out purposes later. There could even be a "strict" mode
that generates errors for unimplemented specs, but they should never,
never fail by default on MRI.
- Charlie
Vladimir Sizikov
There was a small follow-up on the IRC channel and rue has already
reverted the always-failing specs for CGI.
I understand the reason why the "non-implemented specs must fail" was
added, but I agree that for conformance/compatibility specs this is
not very convenient. Well, actually, it's very inconvenient.
I'd vote to revert the mkspec behavior to the old one or we'll be
seeing those new failing specs over and over again.
If the behavior when the non-implemented specs should fail is really
needed, then maybe more complicated scheme, proposed in the end of the
Charles' email (with additional command line switches, etc) might be
added.
Thanks,
--Vladimir
On Apr 5, 10:04 am, Charles Oliver Nutter <charles.nut...@sun.com>
wrote:
Ryan Davis
On Apr 5, 2008, at 07:29 , Vladimir Sizikov wrote:
> I'd vote to revert the mkspec behavior to the old one or we'll be
> seeing those new failing specs over and over again.
no, mkspec should stay as-is, but `mspec ci` should filter those out.
This will let us easily track what we have left to work on.
Charles Oliver Nutter
bin/mspec -tr <some ruby 1.8 spec> should not fail, so I think the hard
failures should be removed.
- Charlie
Vladimir Sizikov
I think the cause of this discussion is that there are two rather
different use cases for the rubyspecs. One is to use them as
(internal) tests driving the development, and another one is to use
them as a common/shared compatibility/conformance test suite for
multiple implementations.
Naturally, external users of the (compatibility) test suite expect
that the tests would at least fully pass on the reference
implementation, and any spec failures are the failures of the
implementation under test. Having failures for not written specs makes
little sense in this scenario.
Also, you're saying that 'mspec ci' would filter those out, but how?
Do you intend to keep/create/maintain exclude files for those
failures, and not only for rubinius, but for MRI as well? Do you
expect that all other Ruby implementors would have to maintain those
excludes and that shoul be done for every single implemenation out
there that decided to use rubyspecs? It seems that we're having hard
time trying to "lure" MRI folks to use rubyspecs as is, and it will be
only harder if they would need to struggle with exclusions for
non-implemented specs.
What's the point for this extra work for everybody? So that it's easy
to see which specs are not implemented? Well, it's easy to see right
now too. All the empty files that end with _spec.rb - are not
implemented specs. :)
Yet another point: The specs are written in rspec-compatible format
and it is expected that should be runnable by rspec. Rspec doesn't
have any excludes support at all.
Ideally, I'd like to see that mspec spec/file/blah.rb should always
pass on MRI 1.8.6, as well as rspec spec/file/blah.rb.
The ability to run MRI 100% clean on the specs is very valuable for
everybody, it seems.
Thanks,
--Vladimir
Brian Ford
Lots of good points here, but to simplify things, I'm only responding
to this post, but I'll comment on several points.
On Apr 5, 2:19 pm, Charles Oliver Nutter <charles.nut...@sun.com>
wrote:
#1. The specs will always pass on the "standard" implementation. (See
the lighthouse spec docs for a discussion of this.) The sole exception
will be specs that are wrapped with the ruby_bug guard.
I am adamantly opposed to "pending" specs. A spec has two states, pass
or fail. There was a huge reluctance to include pending in RSpec and I
think it was an error to do so. A pending spec is an attempt to graft
on a very limited metadata to specs. It inappropriately overloads the
meaning of a spec, which is to pass or fail. We have a much richer
mechanism for adding metadata to specs, namely tags. Right now, we
officially recognize two tags, fails and unstable, for the purpose of
the CI runner. However, the tag and associated comment provide
unlimited ability to describe specs, regardless of whether they pass
or fail, and independent of any particular implementation.
One limitation of tagging is that it requires an actual spec
description on which to hang the tag. That's not really a big deal,
however.
We also use a rich set of guards in specs, and these work just fine
with RSpec despite that it really knows nothing about them.
Combining these two gives us a solution to this problem, which I'll
summarize as:
1. provide a set of specs that passes 100% on the standard
implementation.
2. provide a simple mechanism to signpost methods whose specs need
additional attention.
3. provide a simple way to create template spec files.
The mkspec script (which I am in the process of modifying slightly) is
highly desirable for easily creating the skeleton spec file, properly
named, and identify class and instance methods that need specs. The
script can emit a spec like the following for any method missing a
spec.
describe "Class#method" do
it "needs to be reviewed for completeness" do
flunk if whizbang
end
end
(or describe "Class.method" for class methods.)
The whizbang method will return true given a particular option switch
to the mspec runner. Additionally, these signpost specs can be easily
tagged with a single command (mspec tag --add incomplete --pass -e
"needs to be reviewed for completeness").
Thus, the specs will run clean unless you request otherwise. Metadata
can be added to the spec for any implementation that chooses to do so.
The signpost specs are clearly visible and not confused with actual
specs for those methods. The signpost specs can be removed once some
sort of audit is performed.
This is probably not the "perfect" solution, but it at least 1. makes
consistent of our existing machinery; 2. does not introduce otherwise
useless machinery for pending; 3. is 100% compatible with RSpec; 4.
gives us a much needed first step to creating some sort of audit
process for spec completeness.
My planned (and hopefully committed this weekend) modifications to
mkspec include: 1. generating the signpost spec for both instance and
class methods; and 2. generating the signpost specs for existing files
and new files.
Are everyone's concerns addressed by this solution?
Cheers,
Brian
rue
>
> Are everyone's concerns addressed by this solution?
problem, either:
1. No specs that fail on target implementation, ever.
2. Default running mode excludes 'unimplemented.'
3. Specs should be run with -G [whatever] on target.
We should probably try to map out the different
use cases in more detail at some point.
First, though, we need to decide once and for all
_what it is that the spec repository is supposed to
represent_ since these problems crop up because
of divergent perceptions thereof.
I personally could not care less about RSpec mainly
because its syntax is getting further and further out
of hand but it is perhaps a concern for others.
--
E
Brian Ford
On Apr 5, 11:02 pm, rue <registrati...@kittensoft.org> wrote:
> Brian Ford wrote:
>
> > Are everyone's concerns addressed by this solution?
>
> I think it gets too complicated. For this particular
> problem, either:
>
> 1. No specs that fail on target implementation, ever.
synonymous with 'standard' in my post.
> 2. Default running mode excludes 'unimplemented.'
> 3. Specs should be run with -G [whatever] on target.
options for "default" behavior. You should only need e.g. 'mspec ci'
or 'mspec spec/file_spec.rb', etc.
An audit system for the specs will require *something* additional
because it is metadata. There is nothing in the specs themselves that
can signal whether a set of them is a complete description of the
method. Since we already have a system for spec metadata, the simplest
approach is to use it.
> We should probably try to map out the different
> use cases in more detail at some point.
1. Verify that the specs describe the behavior of the 'standard'
implementation.
2. Verify that the 'target' implementation (the implementation running
the specs) conforms to the 'standard' implementation. This means both
as BDD drivers and as a 'verification/regression test' tool.
> First, though, we need to decide once and for all
> _what it is that the spec repository is supposed to
> represent_ since these problems crop up because
> of divergent perceptions thereof.
principle of always running clean on the 'standard' implementation is
essential.
> I personally could not care less about RSpec mainly
> because its syntax is getting further and further out
> of hand but it is perhaps a concern for others.
diverges, we'll have to revisit. Regardless of the opinion of RSpec,
it is the most widely used spec-like framework. Being compatible
simply lowers the barrier and provides other opportunities for folks.
Cheers,
Brian
Charles Oliver Nutter
> This is too much to ask. You should not have to use any additional
> options for "default" behavior. You should only need e.g. 'mspec ci'
> or 'mspec spec/file_spec.rb', etc.
> It represents a solution to the use cases above. As such, the
> principle of always running clean on the 'standard' implementation is
> essential.
Strongly agree on both points. MRI is the reference implementation, and
it should always run green on the specs without any twiddling. And every
spec written should be confirmed to work on MRI **before** committing.
- Charlie
Charles Oliver Nutter
> describe "Class#method" do
> it "needs to be reviewed for completeness" do
> flunk if whizbang
> end
> end
Why not just format the unimplemented description as
describe "Class#method" do
it "HAS NOT BEEN SPECIFIED" do
end
end
It would be pretty clear from mspec -fs which ones needed work.
- Charlie
Vladimir Sizikov
On Sun, Apr 6, 2008 at 5:12 AM, Brian Ford <bri...@gmail.com> wrote:
> 1. provide a set of specs that passes 100% on the standard
> implementation.
> 2. provide a simple mechanism to signpost methods whose specs need
> additional attention.
> 3. provide a simple way to create template spec files.
>
> describe "Class#method" do
> it "needs to be reviewed for completeness" do
> flunk if whizbang
> end
> end
Looks like a great plan, and I'm all for it. As long as MRI passes on
specs with no failures and with no manual configurations - I'm good :)
There is one (minor and non-blocking) issue though. Adding those
template specs might significantly affect the stats of the test run,
and might confuse those who runy mspec with -f s option, since there
will be many specs printed out with wording that they incomplete. Is
there a way to do that "if whizbang" thing before actually defining
the spec? That way, those who don't currently care about incomplete
specs, won't notice them altogether, not in stats, not in the output,
etc.
Thanks,
--Vladimir
Brian Ford
wrote:
like the signpost specs to be more than that. Obviously, if there are
no specs, the specs for that method are incomplete. However, they
could also be incomplete after some specs have been added. Removing
the 'it "needs to be reviewed for completeness"' can be a deliberate,
more formalized step. In the meantime, specs can be added without
clashing.
Brian
Brian Ford
>
> There is one (minor and non-blocking) issue though. Adding those
> template specs might significantly affect the stats of the test run,
> and might confuse those who runy mspec with -f s option, since there
> will be many specs printed out with wording that they incomplete. Is
> there a way to do that "if whizbang" thing before actually defining
> the spec? That way, those who don't currently care about incomplete
> specs, won't notice them altogether, not in stats, not in the output,
> etc.
>
> Thanks,
> --Vladimir
I want to push for is some more formal method of auditing the specs.
The specs would be visible from a -fs run, but not cause any problems
with failures. I really don't want the incompleteness to be out of
sight.
That said, it is possible to make whizbang a block-based guard like
the rest. But the disadvantage is a clunkier looking spec and lack of
default visibility.
As Charles noted, it is also possible to add a spec that does not
fail, and use the runner's pattern matching to list or tag those
specs. That method removes any need for whizbang. This wouldn't cause
any nuisance for the standard implementation and still make the
signpost specs visible.
It's difficult for me to imagine a segment of the users of the specs
who don't care about completeness. The stats count is a transient
issue, as the incomplete specs should not persist for that long.
Brian