| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
Commit 275ac959db060f80e4b8635e1213cc0c2255c732 prevented specs
that did 'describe Puppet::Face[...]' from working correctly.
That is because the instance returned for the describe call
would then differ if face_collection_spec is run before the
spec in question.
The fix is rather than clearing all face state, save and restore
the global state before and after all examples.
Also correcting a mistake in indirection_base_spec where a ivar
wasn't being used between the before and after clauses.
|
|
The face_collection_base spec attempts to restore global state stored in
Puppet::Interface::FaceCollection so that it doesn't interfere with
other specs that may use faces.
It fails to clear any files required by the autoloader, but clears the
@faces member. The result is that if a face is referenced again from
another spec, it doesn't get defined and added to @faces and cannot be
found.
The fix is to clear the state before and after each example.
The problem can be observed with the following:
rspec spec/unit/interface/face_collection_spec.rb spec/unit/application/face_base_spec.rb
Additionally, the indirection_base spec registers a face but does not remove the face
after the examples have fun. The fix is to remove the face from the FaceCollection after
all examples.
|
|
This reverts commit 1b8a7ea2b8e5835c7d563e23609bb74fd84b43ba.
This backs out the change because it causes Jenkins to fail due to its
spec order.
Will investigate a proper solution to both problems.
|
|
The spec attempts to restore global state stored in
Puppet::Interface::FaceCollection so that it doesn't interfere with
other specs that may use faces.
It fails to clear any files required by the autoloader, but clears the
@faces member. The result is that if a face is referenced again from
another spec, it doesn't get defined and added to @faces and cannot be
found.
The fix is to clear the state before and after each example.
The problem can be observed with the following:
rspec spec/unit/interface/face_collection_spec.rb spec/unit/application/face_base_spec.rb
This closes GH-2422.
|
|
spec/spec_helper was missing from three of the specs. This showed up
because spec/unit/pops/containment_spec.rb just happened to come first
in the order of specs to be executed on our Solaris build, and began
causing failures because puppet/pops was being loaded prior to
spec_helper and puppet. It was also causing rspec/mocha setup and
teardown failures, presumably because of rspec configuration being
called after this initial spec was loaded.
This commit ensures that each of these specs requires spec_helper first
so that spec and the puppet environment initialize properly regardless
of whether these specs run first or come later in the spec ordering.
|
|
The way in which the require statements for the puppet/interface module
worked caused a dependency loop that could only be resolved by using the
modules and classes in a very specific order. The tests showed this
problem if they were run in a different order:
/export/home/jenkins/workspace/Puppet Specs Solaris (master)/solaris10-i386/solaris10-i386/lib/puppet/interface/action_builder.rb:128: uninitialized constant Puppet::Interface::Action (NameError)
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `gem_original_require'
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `require'
from /export/home/jenkins/workspace/Puppet Specs Solaris (master)/solaris10-i386/solaris10-i386/lib/puppet/interface/action_manager.rb:2
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `gem_original_require'
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `require'
from /export/home/jenkins/workspace/Puppet Specs Solaris (master)/solaris10-i386/solaris10-i386/lib/puppet/interface.rb:13
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `gem_original_require'
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `require'
from /export/home/jenkins/workspace/Puppet Specs Solaris (master)/solaris10-i386/solaris10-i386/lib/puppet/interface/action.rb:1
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `gem_original_require'
from /opt/csw/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36:in `require'
from /export/home/jenkins/workspace/Puppet Specs Solaris (master)/solaris10-i386/solaris10-i386/spec/unit/interface/action_spec.rb:3
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/configuration.rb:746:in `load'
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/configuration.rb:746:in `load_spec_files'
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/configuration.rb:746:in `map'
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/configuration.rb:746:in `load_spec_files'
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/command_line.rb:22:in `run'
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/runner.rb:69:in `run'
from /opt/csw/lib/ruby/gems/1.8/gems/rspec-core-2.9.0/lib/rspec/core/runner.rb:10:in `autorun'
from /opt/csw/bin/rspec:19
This re-orders the requires so that only the top level module
(Puppet::Interface) requires all of the components that are part of it.
Paired-with: patrick@puppetlabs.com
|
|
Without this patch Ruby 1.9 is still complaining loudly about trying to
parse the spec files. The previous attempt to clean up this problem in
edc3ddf works for Ruby 1.8 but not 1.9.
I'd prefer to remove the shebang lines entirely, but doing so will cause
encoding errors in Ruby 1.9. This patch strives for a happy middle
ground of convincing Ruby it is actually working with Ruby while not
confusing it to think it should exec() to rspec.
This patch is the result of the following command run against the source
tree:
find spec -type f -print0 | \
xargs -0 perl -pl -i -e 's,^\#\!\s?/(.*)rspec,\#! /usr/bin/env ruby,'
|
|
|
|
* 2.7.x:
Use rspec 2.11 compatible block syntax
Conflicts:
spec/integration/faces/ca_spec.rb
spec/integration/network/server/mongrel_spec.rb
spec/unit/application_spec.rb
spec/unit/face/help_spec.rb
spec/unit/network/handler/fileserver_spec.rb
spec/unit/parser/functions/create_resources_spec.rb
spec/unit/provider/nameservice/directoryservice_spec.rb
spec/unit/type/file_spec.rb
spec/unit/type_spec.rb
|
|
In rspec 2.11, expectations on a block must take the form of expect...to or
lambda...should. Other combinations of those are no longer accepted. This
commit converts all mixed cases to use expect...to, as it seems to be the
preferred syntax now.
|
|
Without this patch some spec files are using `ruby -S rspec` and others
are using `rspec`.
We should standardize on a single form of the interpreter used for spec
files.
`ruby -S rspec` is the best choice because it correctly informs editors
such as Vim with Syntastic that the file is a Ruby file rather than an
Rspec file.
|
|
The previous version mostly worked, but wound up showing an odd failure mode
under Ruby 1.9.3 where it didn't seem to restore state. My best guess is that
something caused the older value to be closed over...
In any case this is an equally horrible, but more effective, way to resolve
the same problem. I don't mind *like* either of them, but they work.
Really, the whole thing should be rethought - it may not actually be worth the
level of pain required to test the loader in the unit tests.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
Add `display_global_options` to Face DSL.
Add `environment`, and `modulepath` as display_global_options for existing
module face.
Add `type` for typed settings.
Add display_global_options to exising help and man faces.
|
|
Change Puppet::Interface::Option to prohibit options on Faces that have
the same name as an existing Puppet setting.
Move functionality of Puppet::Util::Setting::StringSetting.setbycli to
Puppet::Util::Settings and deprecate usage of StringSetting.setbycli
because StringSetting provides metadata for the definition of setting,
whereas Setting contains both the value and the origin of the value,
whether :cli, :memory, :application_defaults, etc.
Change ca application and certificate Face to properly handle all
permutations of --dns_alt_names (Puppet setting) and --dns-alt-names
(Face option).
Remove "documentation only" options of :modulepath and :environment from
module Face.
Prior to this commit, Faces could declare options that collided with
existing Puppet settings, which has caused unexpected behavior for the
Face. This commit explicitly prohibits the Face from declaring an
option which is already a Puppet setting. This is a potentially
breaking change for externally developed Faces.
|
|
|
|
Conflicts:
lib/puppet/network/handler/fileserver.rb
spec/unit/provider/package/dpkg_spec.rb
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
When rendering for face actions was implemented it was assumed that all
the options that an action took would go toward determining the data
that the when_invoked block returned, and the rendering would make
decisions based on that data's structure. However, there are times when
the data returned may be the same even with different options, and the
option will determine how it's rendered.
For example, the `module list` action has a --tree option that will
display the installed modules in a tree view based on dependencies. The
`module search` action will be able to highlight the word that was
searched for in the results.
This commit allows the when_rendering blocks to optionally add options
and arguments along with the invoked result. I made the options and
arguments optional since most when_rendering blocks won't need the
them, and having them show up in the block declaration when not used
is confusing. This also means that all faces previously written don't
need changes and this change is backward compatible.
|
|
This patch allows digits to be used in face names. The first character,
however, is not allowed to be a digit. This fixes #9443.
|
|
When we introduced verification of options, we forgot to handle the case that
global options from the Puppet settings system could be passed to the face.
This, in turn, means that the system would fail if you used any of those.
This remediates that, and now these work as expected.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
Rewrite the process of validating and updating the options to fully reflect
the contract - we fail if there are unknown options passed, report nicely
errors of duplicate names, pass only the canonical names to the action code,
and generally enforce nicely.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
Rather than having multiple, separate operations that modify and validate the
arguments to an action, a single pass makes sense. This also means less walks
across the set of data, and a few less expensive method calls in Ruby.
Additionally, we work on a duplicate of the arguments hash rather than
directly modifying the original. Because everything we do is at the top level
key/value mapping, this is sufficient to isolate the original.
While mostly theoretical, we now don't mutilate the hash passed in, so the
user won't get nastily surprised by the fact that we could have done so.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
Part of the "social contract" of Faces, Actions and Options is that the
metadata we collect is authoritative: it covers everything that is possible.
In the initial release we didn't enforce that around options. If you passed
an unknown option in the hash, we just silently ignored it in validation and
made it available down in the action.
Now, instead, we enforce that rule. If you pass an unknown option we raise an
error and complain; anything that gets to the action will be listed in the set
of inspectable options.
Cases that depended on this behaviour to pass arbitrary content in the hash
should be rewritten to move that content down a level: take a hash value for
one option, and use that for your free content.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
When we invoke an action, we parse a set of options. These have a canonical
name, and optionally a set of aliases. For example, :bar might have :b as an
alias to allow a short name to be given.
Previously we would just pass this on as received; if you passed :bar you got
:bar, and if you passed :b you got :b. This works, but means that every
action has to write the same code to extract the appropriate version of an
option from whatever set of aliases might be passed.
Now, instead, we centralize that and always pass options as their canonical
name to the action code. This makes it simpler to work with. (This happens
before any validation, or other user-supplied, code to simplify everything
that handles options.)
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
When we define an action on an older version of a Face, we must be sure to
directly load the core of that version, not just define it with the external
Action(s) that it had.
Otherwise we break our contract, which is that any core Actions for a specific
version will be available to your external Action for as long as we support
that core version.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
When we first touch a Face, we load all the available Actions from disk.
Given they define themselves against a specific version of a Face, they are
automatically available tied to the correct version; this makes it trivially
possible to locate those on demand and return them.
Now, we have the ability to find and, consequently, invoke Actions on older
versions of Faces. We don't load enough context, though: the older face will
only have external Actions defined, not anything core.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
Given the inheritance model for actions, we are sometimes going to need to set
them to 'default' at runtime, rather than during their static declaration.
Add tests to verify that this works correctly, and update the code to ensure
that happens. This gives up caching of the default action, but this should be
an extremely rare operation - pretty much only CLI invocation, really.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
This implement support for options with default values, allowing faces to set
those values when not invoked. This can eliminate substantial duplicate code
from actions, especially when there are face-level options in use.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
This introduces a class representing a semantic version, and
implementing a few of the most common uses of them: validation,
comparison, and finding the greatest available version matching
a range. This refactoring also allows us to easily expand our
matching of version ranges in the future, which is a big plus.
Reviewed-By: Daniel Pittman
|
|
The problem was caused by the fact that the
options method returns a list of options that
treated the aliases as seperate options.
The fix is to only maintain a list of options
and not add all aliases to the options list.
|
|
2.7.x"
This reverts commit b7ee0258ab40478329c20177eda9b250f27ede18, reversing
changes made to 8fe2e555ac3d57f5b6503ffe1a5466db8d6e190a.
|
|
puppet help was reprinting every option once
for every alias that is had.
This fix involves only storing the option.name
in the @options instance var for both face and
actions options. The @options_hash still
maintains the list of options and aliases as its
keys.
Reviewed-by: Daniel Pittman (puppet-dev list)
|
|
We accidentally omitted whitespace between multiple options while building the
synopsis. This fixes that, by introducing a breakable space in the right
location.
Additionally, we extract the code that was 99 percent identical from the face
and action synopsis generators, push it down into the documentation module,
and then invoke it from both places.
This eliminates the duplicate code, allowing me to fix that bug once and have
it apply to both parts of the code; this is pretty much assured to be true
any time we change the synopsis generation.
Reviewed-By: Nick Fagerlund <nick.fagerlund@puppetlabs.com>
|
|
We earlier moved to duplicating Action objects in the Faces subsystem to
ensure they had the correct binding context during execution and introspection.
This was correct, but introduced a bug where we would report both the parent
and child binding as separate entries with duplicate names, in the list of
actions.
This flowed on to the help output, where it would cause every inherited action
to be listed twice: once on the parent, once on the child. (This was actually
worse if the inheritance was deeper: we would duplicate once for every level
between the instance and the origin of the action.)
|
|
Ruby 1.9 is strict about argument arity for methods that are
metaprogrammatically defined. A ton of specs that were setting up
when_invoked didn't pass options even though they should have been.
Reviewed-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
Since we never shipped this in a real release, we don't need to maintain
compatibility. So, remove it entirely from the codebase.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
|
Where we need special support for :for_humans as an alias for :console, call
it out in comments. This makes it clear to someone who wonders why what the
actual underlying purpose of the whole thing is.
Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
|
|
When we hit a syntax error in any face, a whole bunch of unrelated face things
would blow up in horrible ways. Stack traces for all...
Now, instead, we catch that fault but specifically only in the face file
and report it through our error logs, then quietly ignore the face.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
|
When we query for all faces we need to scan the entire Ruby load path, look
for everything that looks like a face, and load it up. That is a fairly
expensive operation, especially on a platform that has slow I/O.
*cough* EC2 *cough*
Because of that we only scan once, and assume that the list is static
thereafter; this works OK out in the field, but sucks in testing where that
global state gets in the way of the rest of our fiddling under the hood.
This resets the '@loaded' member of the collection additionally, which is what
should be done since we have reset the rest of the collection at the same
time.
We don't bother to reset it, as an extra scan during tests is not a problem.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
|
2.7.x
|
|
We used to let actions be declared without the `when_invoked` block, which was
usually a sign of either someone writing their method code direct in action
declaration, or someone forgetting to add their code at all.
This was just let silently by: the error only showed up when you finally tried
to invoke the action, and a NoMethod error was raised by the face.
...except for our own testing. We took advantage of this a whole pile of
times in there; fixing the original UI issue means fixing all those too.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
|
Reviewed-By: Daniel Pittman
|
|
|
|
From the command line, and other facades, it is fairly difficult to call an
action with two aliases of the same option in the invocation, but from the
Ruby API it would be relatively easy to achieve.
This is now checked, and raises an error, so that we don't accidentally have
strange or unusual behaviour coming out of the whole system.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
`before_action` decorations should always resolve in resolution order
from most general (inherited from furthest away) to most specific
(declared on the instance), and should always execute Face-level
option decorations before action-level option decorations.
`after_action` decorations should execute in the opposite order.
Reviewed-By: Daniel Pittman
|
|
|
|
This extends the last of the documentation support, down into options, so they
can be described as expected. In the process we split out the modular docs
API into a full and short version options only want short docs, but the
behaviours are identical to the full version.
|
|
This allows users to write before_action advice that does basic
option validation very easily.
Reviewed-By: Daniel Pittman
|
|
"Invisible glob", or "prefix", version matching means that when you specify a
version string to use you can specify as little as one version number out of
the semantic versioning spec.
Matching is done on the prefix; an omitted number is treated as "anything" in
that slot, and we return the highest matching versioned face by that spec.
For example, given the set of versions: 1.0.0, 1.0.1, 1.1.0, 1.1.1, 2.0.0
The following would be matched:
input matched
1 1.1.1
1.0 1.0.1
1.0.1 1.0.1
1.0.2 fail - no match
1.1 1.1.1
1.1.1 1.1.1
1.2 fail - no match
|
|
This rewrites a block of identical tests down to a little table, then applies
the test over that.
|
