On Sep 16, 2008, at 1:56 PM, Brice Figureau wrote:
>>
>>
>> I'm not opposed to this, and can see why people are interested in
>> this.
>>
>> Just to be clear, this works by basically making a new variable in
>> the
>> current scope that is a concatenation of the specified variable and
>> the new text/array, right?
>
> Not a new variable, it adds in the current scope a variable with the
> same name as the upper scope variable but whose value is concatenated
> with the current rvalue.
>
> The implementation in the patch above allows to append _in the same
> scope_, but I don't think this is right, hence my aforementioned
> comment
> that I can easily explicitely allow append operator only on new
> descendant context.
Sorry, i didn't reply to that earlier -- I, too, think appending
should only be allowed in a new scope, not in the same scope.
>
>> If so, then that seems perfectly reasonable to me, since it's very
>> similar to just using something like:
>>
>> $othervar = "$var new text"
>>
>> It only affects lower scopes, rather than modifying the original
>> value, so it should work great.
>
> Yes.
>
>> As to tests, there should be sufficient lexer tests, but I agree that
>> we don't have parser tests to speak of; all I've got are integration
>> tests in the form of snippets, like you already have done. It might
>> be a good time to create that parser unit test file and add tests for
>> this code. :)
>
> My problem is that I don't even know how to start a parser unit
> rspec :-(
>
> The only thing I can think of is: parse some puppet snippets (or
> should
> I feed lexer results?), then mock the AST::XYZ object to check that
> they
> are called fine. Is that the right thing to do?
>
> Right now, I have a rspec append test, that is more an integration
> test,
> as it feeds puppet snippets to a parser, then compile the AST tree,
> then
> finally checks that the scope contains the right value, or that right
> scope function setvar is called. Is that something you might include?
Well, there are a couple of ways you can do this, but what I'd
generally recommend is splitting all of the tests into the different
classes.
You're changing three things: an AST::VarDef class, the scope class,
and the Parser class. I'd normally add tests for each of these three
classes.
The AST and scope classes are straightforward to test, but ask for
more detail if you'd like help.
The Parser class is a bit harder to test, but really, all you need to
test is that a given chunk of text results in the appropriate AST
instances being created. E.g., something like (this code won't work,
but it's off the top of my head):
parser = Parser.new
AST::VarDef.expects(:new).with { |scope, otherstuff, append| append ==
true }
parser.parse("$var += something")
In other words, you just mock that the right AST instances are being
created with the right options.
I will say that this stuff isn't necessarily easy to test, and I'm
totally behind any refactoring that might make sense to make it easier.
>
>> I think the major changes, though, are in the scope class, and it's
>> also only tested in test/ rather than spec/.
>
> OK, so I can also start a rspec scope that at least check the setvar
> method.
>
> On a different side, there are demand for more complex if test (ie
> including or, and, ==, !=, etc ala collection expressions). I'm
> volunteer to implement this (and even already started). Is this
> something that you can include, or should I use my spare time for
> something else?
I'd include them if they were available, yeah, so go ahead and do
them, as long as you're doing tests at the same time.
I'd *much* prefer to help you get the tests done right than worry
about whether I'd accept your code later because maybe the tests
aren't sufficient later, btw. You seem like quite the self-starter,
so I'm trying to point you in the right direction and let you kick
ass, but if you want help, just let me know and I'll do what I can.
--
The only really good place to buy lumber is at a store where the lumber
has already been cut and attached together in the form of furniture,
finished, and put inside boxes. --Dave Barry
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
