codenarc-developer

[Codenarc-developer] Inline violations idea

[Artur G. <art...@gm...>]

Hi!

I think I've got an idea for a significant improvement of the (already
great) CodeNarc's test framework and would like to consult it before making
a pull request ;)

The thing that is (for me) the most cumbersome when writing tests is
passing the line number and line text to the assert*Violation[s] methods. I
think I understand the rationale behind having to pass both of them, which
seems to be a kind of a double check / 'checksum'. I'd like to propose an
API in which we only have to write the violation message - and the API
reliably finds the sourceLineText and lineNumber for us. I've made a POC
implementation in
inlineViolations<https://github.com/ArturGajowy/CodeNarc/tree/inlineViolations>
branch
of my fork. The result is best depicted
here<https://github.com/ArturGajowy/CodeNarc/commit/9cb0777e9f1c14d4b532bbd43d0e84bea6ecf494#commitcomment-3147447>
.

As you can see, the whole idea is based on violation markers placed in the
test sources - just in the line the violation takes place. This way, when
we add a line to the source before the violation (e.g. an import we've
missed or a class header, or another violating line) - we don't have to
edit any preexisting violation assertions. There's also no copy'n'paste and
line counting invlolved in the initial process of writing tests :)

The violation markers are removed before the analysis takes place, so that
the assertion errors do not become illegible and the markers don't
interfere with analysis made by rule under test.

As I've said - this is a POC. It lacks: support for multiple violations per
line, standalone tests, a bit of defensive programming and first of all -
your approval of the API :) Let me know what do you think of the idea: do
you like it, would you like this in CodeNarc, are there any issues to
address before you'd like a pull request with that.

And of course - I'll be more than happy to implement the final version :)

Best regards,
Artur Gajowy

Re: [Codenarc-developer] Inline violations idea

[Chris M. <chr...@ea...>]

Thanks Artur. That is a clever and innovative approach for validation of
rule violations. Pretty cool! I acknowledge that configuring the violation
line numbers and source line text and keeping them in sync after edits can
be cumbersome. Your solution is a concise way to avoid that.

 

Some things to consider about the existing approach:

.         The existing approach and associated validations are there
intentionally to validate the exact line number in the generated Violation;
line numbers are not always completely straightforward and automatically
correct (especially around imports and annotations). Though the CodeNarc
framework and helper classes generally make that a non-issue now, I would
not want to skip line number validation for all rules/tests.

.         The source line text is not guaranteed to be the full source line
text; it may be a subset, or it may be null/empty. So, you could not
generalize that it is always the full source line.

.         I do like the way the existing approach allows a clean, standalone
example of source code that causes violation. It can also be easily copied
to the help documentation or pasted in an email (admittedly a minor
benefit).

 

That all being said, I do think your approach has value. I would want to
implement this as a separate method name, perhaps assertInlineViolations(),
rather than adding complexity (and potentially parsing the source code?) for
all invocations of assertViolations(). If you are willing to expand and
harden your POC as you mentioned, I think we can pull it in.

 

Thanks.

Chris

Re: [Codenarc-developer] Inline violations idea

[Marcin E. <mar...@pr...>]

I'm a bit late to the party but I really like the idea. I never wrote 
too many rules but when I did line counting and passing those maps to 
assertViolations() was the biggest pain point in the otherwise terrific 
testing support for writing rules in Codenarc. So it would be nice if 
you pushed on and made all those changes required by Chris so that he 
has no choice but to merge it in. :)

On 07/05/13 11:44, Chris Mair wrote:
>
> Thanks Artur. That is a clever and innovative approach for validation 
> of rule violations. Pretty cool! I acknowledge that configuring the 
> violation line numbers and source line text and keeping them in sync 
> after edits can be cumbersome. Your solution is a concise way to avoid 
> that.
>
> Some things to consider about the existing approach:
>
> ·The existing approach and associated validations are there 
> intentionally to validate the exact line number in the generated 
> Violation; line numbers are not always completely straightforward and 
> automatically correct (especially around imports and annotations). 
> Though the CodeNarc framework and helper classes generally make that a 
> non-issue now, I would not want to skip line number validation for all 
> rules/tests.
>
> ·The source line text is not guaranteed to be the full source line 
> text; it may be a subset, or it may be null/empty. So, you could not 
> generalize that it is always the full source line.
>
> ·I do like the way the existing approach allows a clean, standalone 
> example of source code that causes violation. It can also be easily 
> copied to the help documentation or pasted in an email (admittedly a 
> minor benefit).
>
> That all being said, I do think your approach has value. I would want 
> to implement this as a separate method name, perhaps 
> *assertInlineViolations*(), rather than adding complexity (and 
> potentially parsing the source code?) for all invocations of 
> *assertViolations*(). If you are willing to expand and harden your POC 
> as you mentioned, I think we can pull it in.
>
> Thanks.
>
> Chris
>
>
>
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and
> their applications. This 200-page book is written by three acclaimed
> leaders in the field. The early access version is available now.
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
>
>
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer

Re: [Codenarc-developer] Inline violations idea

[Hamlet D. <ham...@ca...>]

The biggest issue is that the getLineNumber() method in Groovy is often returning the wrong line number. As long as the line number hacks in CodeNarc's AstUtils are well tested, then it seems like a reasonable change. However, I would not have two different public methods on the API. I don't think many projects depend on these test utilities, and it would be a lot cleaner to just have one way to make assertions rather than two. So I would go ahead and break backwards compatibility if we really do believe that this is a better API for testing. 

-- 
Hamlet D'Arcy 
ham...@ca... 

----- Original Message -----

> I'm a bit late to the party but I really like the idea. I never wrote
> too many rules but when I did line counting and passing those maps
> to assertViolations() was the biggest pain point in the otherwise
> terrific testing support for writing rules in Codenarc. So it would
> be nice if you pushed on and made all those changes required by
> Chris so that he has no choice but to merge it in. :)

> On 07/05/13 11:44, Chris Mair wrote:

> > Thanks Artur. That is a clever and innovative approach for
> > validation
> > of rule violations. Pretty cool! I acknowledge that configuring the
> > violation line numbers and source line text and keeping them in
> > sync
> > after edits can be cumbersome. Your solution is a concise way to
> > avoid that.
> 

> > Some things to consider about the existing approach:
> 
> > · The existing approach and associated validations are there
> > intentionally to validate the exact line number in the generated
> > Violation; line numbers are not always completely straightforward
> > and automatically correct (especially around imports and
> > annotations). Though the CodeNarc framework and helper classes
> > generally make that a non-issue now, I would not want to skip line
> > number validation for all rules/tests.
> 
> > · The source line text is not guaranteed to be the full source line
> > text; it may be a subset, or it may be null/empty. So, you could
> > not
> > generalize that it is always the full source line.
> 
> > · I do like the way the existing approach allows a clean,
> > standalone
> > example of source code that causes violation. It can also be easily
> > copied to the help documentation or pasted in an email (admittedly
> > a
> > minor benefit).
> 

> > That all being said, I do think your approach has value. I would
> > want
> > to implement this as a separate method name, perhaps
> > assertInlineViolations (), rather than adding complexity (and
> > potentially parsing the source code?) for all invocations of
> > assertViolations (). If you are willing to expand and harden your
> > POC as you mentioned, I think we can pull it in.
> 

> > Thanks.
> 
> > Chris
> 

> > ------------------------------------------------------------------------------
> 
> > Learn Graph Databases - Download FREE O'Reilly Book
> 
> > "Graph Databases" is the definitive new guide to graph databases
> > and
> 
> > their applications. This 200-page book is written by three
> > acclaimed
> 
> > leaders in the field. The early access version is available now.
> 
> > Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
> 

> > _______________________________________________
> 
> > Codenarc-developer mailing list
> > Cod...@li...
> > https://lists.sourceforge.net/lists/listinfo/codenarc-developer
> 
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and
> their applications. This 200-page book is written by three acclaimed
> leaders in the field. The early access version is available now.
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer

Re: [Codenarc-developer] Inline violations idea

[Chris M. <chr...@ea...>]

I’m glad we are getting some discussion and feedback on this.

 

I don’t think there is any question about breaking backwards-compatibility. Artur’s changes integrate with and are compatible with the existing APIs. We would continue to support new and old validation styles. I don’t think anyone is suggesting migrating all of the existing code to the new style (and potentially breaking other rule developers’ code), but correct me if I’m wrong.

 

I am still leaning toward a separate method for reasons including:

·         This is a different style of validation; I think making that obvious is beneficial.

·         I am worried about the potential for confusing the new and old styles of validations. For instance, a developer intends to use the new style , but neglects to annotate the source code with validations, and the test succeeds, even if it “should” not.

·         There may be design/complexity advantages to keeping the new and old styles decoupled. That may give us more flexibility to extend or redesign later.

·         It would just be adding a single protected method to the AbstractRuleTestCase API.

 

Chris

 

From: Hamlet DArcy [mailto:ham...@ca...] 
Sent: Friday, May 10, 2013 3:22 AM
To: Marcin Erdmann
Cc: cod...@li...
Subject: Re: [Codenarc-developer] Inline violations idea

 

The biggest issue is that the getLineNumber() method in Groovy is often returning the wrong line number. As long as the line number hacks in CodeNarc's AstUtils are well tested, then it seems like a reasonable change. However, I would not have two different public methods on the API. I don't think many projects depend on these test utilities, and it would be a lot cleaner to just have one way to make assertions rather than two. So I would go ahead and break backwards compatibility if we really do believe that this is a better API for testing. 

--
Hamlet D'Arcy
ham...@ca...

 

  _____  

I'm a bit late to the party but I really like the idea. I never wrote too many rules but when I did line counting and passing those maps to assertViolations() was the biggest pain point in the otherwise terrific testing support for writing rules in Codenarc. So it would be nice if you pushed on and made all those changes required by Chris so that he has no choice but to merge it in. :)

On 07/05/13 11:44, Chris Mair wrote:

Thanks Artur. That is a clever and innovative approach for validation of rule violations. Pretty cool! I acknowledge that configuring the violation line numbers and source line text and keeping them in sync after edits can be cumbersome. Your solution is a concise way to avoid that.

 

Some things to consider about the existing approach:

·         The existing approach and associated validations are there intentionally to validate the exact line number in the generated Violation; line numbers are not always completely straightforward and automatically correct (especially around imports and annotations). Though the CodeNarc framework and helper classes generally make that a non-issue now, I would not want to skip line number validation for all rules/tests.

·         The source line text is not guaranteed to be the full source line text; it may be a subset, or it may be null/empty. So, you could not generalize that it is always the full source line.

·         I do like the way the existing approach allows a clean, standalone example of source code that causes violation. It can also be easily copied to the help documentation or pasted in an email (admittedly a minor benefit).

 

That all being said, I do think your approach has value. I would want to implement this as a separate method name, perhaps assertInlineViolations(), rather than adding complexity (and potentially parsing the source code?) for all invocations of assertViolations(). If you are willing to expand and harden your POC as you mentioned, I think we can pull it in.

 

Thanks.

Chris





------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and 
their applications. This 200-page book is written by three acclaimed 
leaders in the field. The early access version is available now. 
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may





_______________________________________________
Codenarc-developer mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

_______________________________________________
Codenarc-developer mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

 

Re: [Codenarc-developer] Inline violations idea

[Chris M. <chr...@ea...>]

Artur,

 

Are you still planning on submitting the changes for the inline violations support? Let me know if there is anything I can do to help. Thanks.

 

Chris

 

From: Chris Mair [mailto:chr...@ea...] 
Sent: Friday, May 10, 2013 9:30 PM
To: 'Hamlet DArcy'; 'Marcin Erdmann'
Cc: 'cod...@li...'
Subject: RE: [Codenarc-developer] Inline violations idea

 

I’m glad we are getting some discussion and feedback on this.

 

I don’t think there is any question about breaking backwards-compatibility. Artur’s changes integrate with and are compatible with the existing APIs. We would continue to support new and old validation styles. I don’t think anyone is suggesting migrating all of the existing code to the new style (and potentially breaking other rule developers’ code), but correct me if I’m wrong.

 

I am still leaning toward a separate method for reasons including:

·         This is a different style of validation; I think making that obvious is beneficial.

·         I am worried about the potential for confusing the new and old styles of validations. For instance, a developer intends to use the new style , but neglects to annotate the source code with validations, and the test succeeds, even if it “should” not.

·         There may be design/complexity advantages to keeping the new and old styles decoupled. That may give us more flexibility to extend or redesign later.

·         It would just be adding a single protected method to the AbstractRuleTestCase API.

 

Chris

 

From: Hamlet DArcy [mailto:ham...@ca...] 
Sent: Friday, May 10, 2013 3:22 AM
To: Marcin Erdmann
Cc: cod...@li...
Subject: Re: [Codenarc-developer] Inline violations idea

 

The biggest issue is that the getLineNumber() method in Groovy is often returning the wrong line number. As long as the line number hacks in CodeNarc's AstUtils are well tested, then it seems like a reasonable change. However, I would not have two different public methods on the API. I don't think many projects depend on these test utilities, and it would be a lot cleaner to just have one way to make assertions rather than two. So I would go ahead and break backwards compatibility if we really do believe that this is a better API for testing. 

--
Hamlet D'Arcy
ham...@ca...

 

  _____  

I'm a bit late to the party but I really like the idea. I never wrote too many rules but when I did line counting and passing those maps to assertViolations() was the biggest pain point in the otherwise terrific testing support for writing rules in Codenarc. So it would be nice if you pushed on and made all those changes required by Chris so that he has no choice but to merge it in. :)

On 07/05/13 11:44, Chris Mair wrote:

Thanks Artur. That is a clever and innovative approach for validation of rule violations. Pretty cool! I acknowledge that configuring the violation line numbers and source line text and keeping them in sync after edits can be cumbersome. Your solution is a concise way to avoid that.

 

Some things to consider about the existing approach:

·         The existing approach and associated validations are there intentionally to validate the exact line number in the generated Violation; line numbers are not always completely straightforward and automatically correct (especially around imports and annotations). Though the CodeNarc framework and helper classes generally make that a non-issue now, I would not want to skip line number validation for all rules/tests.

·         The source line text is not guaranteed to be the full source line text; it may be a subset, or it may be null/empty. So, you could not generalize that it is always the full source line.

·         I do like the way the existing approach allows a clean, standalone example of source code that causes violation. It can also be easily copied to the help documentation or pasted in an email (admittedly a minor benefit).

 

That all being said, I do think your approach has value. I would want to implement this as a separate method name, perhaps assertInlineViolations(), rather than adding complexity (and potentially parsing the source code?) for all invocations of assertViolations(). If you are willing to expand and harden your POC as you mentioned, I think we can pull it in.

 

Thanks.

Chris

 

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and 
their applications. This 200-page book is written by three acclaimed 
leaders in the field. The early access version is available now. 
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may

 

_______________________________________________
Codenarc-developer mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

_______________________________________________
Codenarc-developer mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

 

Re: [Codenarc-developer] Inline violations idea

[Artur G. <art...@gm...>]

Chris,

Sure! I just have to find some time to make the changes :) I can't really
promise anything, but will try to make them within a month.
I think you can't do much to help me for now, but thanks for asking!

Best regards,
Artur

On 4 July 2013 16:06, Chris Mair <chr...@ea...> wrote:

> Artur,****
>
> ** **
>
> Are you still planning on submitting the changes for the inline violations
> support? Let me know if there is anything I can do to help. Thanks.****
>
> ** **
>
> Chris****
>
> ** **
>
> *From:* Chris Mair [mailto:chr...@ea...]
> *Sent:* Friday, May 10, 2013 9:30 PM
> *To:* 'Hamlet DArcy'; 'Marcin Erdmann'
> *Cc:* 'cod...@li...'
> *Subject:* RE: [Codenarc-developer] Inline violations idea****
>
> ** **
>
> I’m glad we are getting some discussion and feedback on this.****
>
> ** **
>
> I don’t think there is any question about breaking
> backwards-compatibility. Artur’s changes integrate with and are compatible
> with the existing APIs. We would continue to support new and old validation
> styles. I don’t think anyone is suggesting migrating all of the existing
> code to the new style (and potentially breaking other rule developers’
> code), but correct me if I’m wrong.****
>
> ** **
>
> I am still leaning toward a separate method for reasons including:****
>
> **·         **This is a different style of validation; I think making
> that obvious is beneficial.****
>
> **·         **I am worried about the potential for confusing the new and
> old styles of validations. For instance, a developer intends to use the new
> style , but neglects to annotate the source code with validations, and the
> test succeeds, even if it “should” not.****
>
> **·         **There may be design/complexity advantages to keeping the
> new and old styles decoupled. That may give us more flexibility to extend
> or redesign later.****
>
> **·         **It would just be adding a single protected method to the *
> AbstractRuleTestCase* API.****
>
> ** **
>
> Chris****
>
> ** **
>
> *From:* Hamlet DArcy [mailto:ham...@ca...<ham...@ca...>]
>
> *Sent:* Friday, May 10, 2013 3:22 AM
> *To:* Marcin Erdmann
> *Cc:* cod...@li...
> *Subject:* Re: [Codenarc-developer] Inline violations idea****
>
> ** **
>
> The biggest issue is that the getLineNumber() method in Groovy is often
> returning the wrong line number. As long as the line number hacks in
> CodeNarc's AstUtils are well tested, then it seems like a reasonable
> change. However, I would not have two different public methods on the API.
> I don't think many projects depend on these test utilities, and it would be
> a lot cleaner to just have one way to make assertions rather than two. So I
> would go ahead and break backwards compatibility if we really do believe
> that this is a better API for testing. ****
>
> --
> Hamlet D'Arcy
> ham...@ca...****
>
> ** **
> ------------------------------
>
> I'm a bit late to the party but I really like the idea. I never wrote too
> many rules but when I did line counting and passing those maps to
> assertViolations() was the biggest pain point in the otherwise terrific
> testing support for writing rules in Codenarc. So it would be nice if you
> pushed on and made all those changes required by Chris so that he has no
> choice but to merge it in. :)
>
> On 07/05/13 11:44, Chris Mair wrote:****
>
> Thanks Artur. That is a clever and innovative approach for validation of
> rule violations. Pretty cool! I acknowledge that configuring the violation
> line numbers and source line text and keeping them in sync after edits can
> be cumbersome. Your solution is a concise way to avoid that.****
>
>  ****
>
> Some things to consider about the existing approach:****
>
> ·         The existing approach and associated validations are there
> intentionally to validate the exact line number in the generated Violation;
> line numbers are not always completely straightforward and automatically
> correct (especially around imports and annotations). Though the CodeNarc
> framework and helper classes generally make that a non-issue now, I would
> not want to skip line number validation for all rules/tests.****
>
> ·         The source line text is not guaranteed to be the full source
> line text; it may be a subset, or it may be null/empty. So, you could not
> generalize that it is always the full source line.****
>
> ·         I do like the way the existing approach allows a clean,
> standalone example of source code that causes violation. It can also be
> easily copied to the help documentation or pasted in an email (admittedly a
> minor benefit).****
>
>  ****
>
> That all being said, I do think your approach has value. I would want to
> implement this as a separate method name, perhaps *assertInlineViolations*(),
> rather than adding complexity (and potentially parsing the source code?)
> for all invocations of *assertViolations*(). If you are willing to expand
> and harden your POC as you mentioned, I think we can pull it in.****
>
>  ****
>
> Thanks.****
>
> Chris****
>
> ** **
>
> ------------------------------------------------------------------------------****
>
> Learn Graph Databases - Download FREE O'Reilly Book****
>
> "Graph Databases" is the definitive new guide to graph databases and ****
>
> their applications. This 200-page book is written by three acclaimed ****
>
> leaders in the field. The early access version is available now. ****
>
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may****
>
> ** **
>
> _______________________________________________****
>
> Codenarc-developer mailing list****
>
> Cod...@li...****
>
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer****
>
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer****
>
> ** **
>