On Mon, 2008-10-20 at 19:22 -0500, Luke Kanies wrote:
> On Oct 12, 2008, at 4:25 AM, Brice Figureau wrote:
> 
> >
> > Hi,
> >
> > Sorry if this ends by a double post, I sent it first with my regular
> > e-mail address which is not subscribed to this list.
> > Note, that I never received an e-mail I git sended yesterday whose
> > subject starts with [PATCH 1/2] Fix #1088. I received the other two  
> > but
> > this one seems to be stuck somewhere. I can see it in the web archive
> > though, so that's quite strange (that's not the first time some google
> > groups mails takes a long time to reach me).
> >
> > On 11/10/08 22:36, Luke Kanies wrote:
> >> +1, but with comments below.
> >>
> >> On Oct 11, 2008, at 1:00 PM, Brice Figureau wrote:
> >>> diff --git a/test/data/snippets/multipleclass.pp b/test/data/
> >>> snippets/multipleclass.pp
> >>> new file mode 100644
> >>> index 0000000..fb363a1
> >>> --- /dev/null
> >>> +++ b/test/data/snippets/multipleclass.pp
> >>> @@ -0,0 +1,9 @@
> >>> +class one {
> >>> +notice('class one')
> >>> +}
> >>> +
> >>> +class one {
> >>> +notice('second class one')
> >>> +}
> >>> +
> >>> +include one
> >>
> >>
> >> Shouldn't there be a test somewhere for this?  Snippets without an
> >> associated test method in test/language/snippets are ignored.
> >
> > Actually I didn't want to commit this file. It was my test file for
> > debugging the issue. It just happens this directory is handy to drop
> > such files.
> >
> > Anyway, I just pushed a rewrite of this file and the corresponding
> > snippet test
> > I'll update the other not merged tickets I just fixed that contains
> > snippets with snippet tests, if you think it is necessary.
> 
> At the least, each snippet should have an empty test, since otherwise  
> the snippet is totally ignored.  With an empty test, the snippet is at  
> least syntax-checked.

The snippets are also used in lexer test, so snippets using a "new"
token don't absolutely need a snippet test. But you're right, I've added
snippets tests to the code that wasn't yet checked in in 0.24.x. If I
have time, I'll review my older submissions to see if I can add more
snippets test.

> I'm not exactly thrilled with those tests, esp. since they all require  
> transactions and everything -- it'd be much better to test the  
> resulting catalog.

Yes. I think so. 
Not really related, but maybe you could offer me a good advice, is that
the main issue I have with rspec parser test is how to test that the
produced ast tree is correct in case of complex expression. I can
"expect" the various new method called, but I'd like to make sure the
returned structure matches what the parser tree should be. 

> In the meantime, though, it'd be best to have a snippet test for each  
> of the syntax types.

I'll see if I can add more snippets in an upcoming patch.
-- 
Brice Figureau <[EMAIL PROTECTED]>


--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to