Re: t/codingstd/perlcritic.t needs to be optional
From: Patrick R. Michaud [EMAIL PROTECTED] Date: Fri, 29 Jun 2007 13:42:18 -0500 On Fri, Jun 29, 2007 at 05:59:38PM +0100, Nicholas Clark wrote: Modified since when? Since the last time the user ran Configure. (For the default test run) I think that this will produce minimal false positives and false negatives, for identifying which files have been locally edited. This might work for others -- it probably wouldn't help me. I tend to run make realclean; perl Configure.pl; before doing a make test prior to checkin... I do pretty much the same thing, come to think of it. How about a dummy perlcritic-timestamp file that gets touched as the last step of Configure.pl (and not walloped by make realclean), so that there is a single place to reset? Or maybe it should be touched only if it doesn't already exist, and we can delete it when the load creeps up? -- Bob Rogers http://rgrjr.dyndns.org/ [Resent, since it's been more than 30H since the first post.]
Re: t/codingstd/perlcritic.t needs to be optional
Modified since when? Since the last time the user ran Configure. (For the default test run) I think that this will produce minimal false positives and false negatives, for identifying which files have been locally edited. Nicholas Clark
Re: t/codingstd/perlcritic.t needs to be optional
On Fri, Jun 29, 2007 at 05:59:38PM +0100, Nicholas Clark wrote: Modified since when? Since the last time the user ran Configure. (For the default test run) I think that this will produce minimal false positives and false negatives, for identifying which files have been locally edited. This might work for others -- it probably wouldn't help me. I tend to run make realclean; perl Configure.pl; before doing a make test prior to checkin... just to make sure I'm working from something clean. So, checking files modified since configure would tend to miss the files I just changed. Pm
Re: t/codingstd/perlcritic.t needs to be optional
From: Patrick R. Michaud [EMAIL PROTECTED] Date: Fri, 29 Jun 2007 13:42:18 -0500 On Fri, Jun 29, 2007 at 05:59:38PM +0100, Nicholas Clark wrote: Modified since when? Since the last time the user ran Configure. (For the default test run) I think that this will produce minimal false positives and false negatives, for identifying which files have been locally edited. This might work for others -- it probably wouldn't help me. I tend to run make realclean; perl Configure.pl; before doing a make test prior to checkin... I do pretty much the same thing, come to think of it. How about a dummy perlcritic-timestamp file that gets touched as the last step of Configure.pl (and not walloped by make realclean), so that there is a single place to reset? Or maybe it should be touched only if it doesn't already exist, and we can delete it when the load creeps up? -- Bob Rogers http://rgrjr.dyndns.org/
Re: t/codingstd/perlcritic.t needs to be optional
On Jun 28, 2007, at 12:52 AM, chromatic wrote: Heck, you didn't even *compile* before one of your checkins yesterday. Yeah, I did. I just had modified something else on a wild tear, forgot to revert it, and did a commit hours later. Sorry about that. -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: t/codingstd/perlcritic.t needs to be optional
On Jun 28, 2007, at 12:52 AM, chromatic wrote: Heck, you didn't even *compile* before one of your checkins yesterday. And really, this speaks even more about where the check should be. What if we have the Perl::Critic checks as Subversion commit hooks? Could email p6i with the results, too. That's what we do at work, and it's annoying, but it's there and it's pretty in-your-face. xoxo, Andy -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: t/codingstd/perlcritic.t needs to be optional
On Wed, Jun 27, 2007 at 10:52:49PM -0700, chromatic wrote: On Wednesday 27 June 2007 22:38:17 Andy Lester wrote: It'd have to be against the last update from svn of the file itself. Yes. ...just to toss some random brainstorms into the mix here... To avoid svn-specific behavior, is there perhaps another file in the repository that we could use to compare timestamp against? I personally don't have an issue with tying things to svn -- I just think we might be able to do it otherwise. Even possibly check files modified within the last NN hours. Also, instead of running/mailing the perlcritic tests on every checkin, we could perhaps set up a cron job to do it once per day. This would keep potential mail messages down, and may be easier to set up and control than a subversion hook. (I could fairly quickly set up such a system as part of the daily smoke tests that I already run from my box.) Again, just some random ideas... Pm
Re: t/codingstd/perlcritic.t needs to be optional
chromatic schrieb:
On Wednesday 27 June 2007 13:22:22 Andy Lester wrote:
The Perl::Critic testing in t/codingstd/perlcritic.t needs to be
optional. The existence of Perl::Critic on a machine doesn't mean
that it's appropriate to run Perl::Critic on the Parrot code.
I'd like to see an option to run it only on *modified* files. I'm not aware
of a mechanism where source code mysteriously changes without the presence of
cosmic rays, and I'm not aware of any testing mechanisms reliable in the face
of cosmic rays.
Actually it seemed to have happened at my company,
http://use.perl.org/~Bernhard/journal/33593
Suddenly a space had changed into a '(' in Sys::Hostname. However It
didn't need Perl::Critic to uncover that.
Regards,
Bernhard
Re: t/codingstd/perlcritic.t needs to be optional
# from Andy Lester # on Wednesday 27 June 2007 10:09 pm: Modified since when? Create a .critictest file when it succeeds and use that timestamp? # from chromatic # on Wednesday 27 June 2007 11:10 pm: What if we have the Perl::Critic checks as Subversion commit hooks? Could email p6i with the results, too. That's what we do at work, and it's annoying, but it's there and it's pretty in-your-face. If it doesn't hose svk push, where the first of several commits fails due to standards violations, I wouldn't mind trying it as an experiment. I would guess a post-commit hook run in the background would be the way to do it. Besides the timeout issue, complex code preventing a checkin may be a bad thing because one would need to track-down a server admin if the gatekeeper script had a bug. If the tests run quickly enough locally, that will encourage clean checkins, with the post-commit informing the list of any dirty ones. --Eric -- You can't win. You can't break even. You can't quit. --Ginsberg's Restatement of the Three Laws of Thermodynamics --- http://scratchcomputing.com ---
Re: t/codingstd/perlcritic.t needs to be optional
Andy Lester wrote: Modified since when? It'd have to be against the last update from svn of the file itself. I'm not sure I like the idea of relying on a given VCS. I know Parrot's hosted in Subversion, but what about the Git folks? Perhaps a better approach is to squirrel away an MD5 of the source file last time it was checked (or remembers all MD5s of all good source files, irrespective of whether they actually exist in the repository as latest versions -- that would help with the Git issues). Of course, this would mean checking in a generated file; but perhaps that could be done by a robot that sweeps the tree once a day or so. An alternative to checking in the MD5 list would be to use a queryable server; but some people work offline or have high-latency links, so perhaps that doesn't work so well.
t/codingstd/perlcritic.t needs to be optional
The Perl::Critic testing in t/codingstd/perlcritic.t needs to be optional. The existence of Perl::Critic on a machine doesn't mean that it's appropriate to run Perl::Critic on the Parrot code. xoa -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: t/codingstd/perlcritic.t needs to be optional
On Jun 27, 2007, at 3:22 PM, Andy Lester wrote: The Perl::Critic testing in t/codingstd/perlcritic.t needs to be optional. The existence of Perl::Critic on a machine doesn't mean that it's appropriate to run Perl::Critic on the Parrot code. Following up, it takes almost 11 unresponsive minutes to run t/ codingstd/perlcritic.t on my box. Maybe it's just my system that's being butt slow on it. Anyone else getting times like this? -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: t/codingstd/perlcritic.t needs to be optional
Andy Lester wrote: Maybe it's just my system that's being butt slow on it. Anyone else getting times like this? -- On my Linux Virtual Machine: [li11-226:parrot] 503 $ time perl t/codingstd/perlcritic.t # Perl::Critic::Bangs not installed: not testing for TODO items in code 1..9 ok 1 - CodeLayout::ProhibitDuplicateCoda ok 2 - CodeLayout::ProhibitHardTabs ok 3 - CodeLayout::ProhibitTrailingWhitespace ok 4 - CodeLayout::UseParrotCoda ok 5 - TestingAndDebugging::MisplacedShebang ok 6 - TestingAndDebugging::ProhibitShebangWarningsArg ok 7 - TestingAndDebugging::RequirePortableShebang ok 8 - TestingAndDebugging::RequireUseStrict ok 9 - TestingAndDebugging::RequireUseWarnings real0m45.128s user0m33.120s sys 0m0.030s On my iBook G4: [parrot] 505 $ time perl t/codingstd/perlcritic.t # Perl::Critic::Bangs not installed: not testing for TODO items in code # Using tools/util/perltidy.conf for Perl::Tidy settings 1..9 ok 1 - CodeLayout::ProhibitDuplicateCoda ok 2 - CodeLayout::ProhibitHardTabs ok 3 - CodeLayout::ProhibitTrailingWhitespace ok 4 - CodeLayout::UseParrotCoda ok 5 - TestingAndDebugging::MisplacedShebang ok 6 - TestingAndDebugging::ProhibitShebangWarningsArg ok 7 - TestingAndDebugging::RequirePortableShebang ok 8 - TestingAndDebugging::RequireUseStrict ok 9 - TestingAndDebugging::RequireUseWarnings real3m6.533s user2m36.992s sys 0m3.880s I think it's mostly your box. However, I agree that to spend 1 minute with no message on STDOUT is annoying ... and moreso if that's at the end of a long 'make test' suite. kid51
Re: t/codingstd/perlcritic.t needs to be optional
On Wednesday 27 June 2007 13:22:22 Andy Lester wrote: The Perl::Critic testing in t/codingstd/perlcritic.t needs to be optional. The existence of Perl::Critic on a machine doesn't mean that it's appropriate to run Perl::Critic on the Parrot code. I'd like to see an option to run it only on *modified* files. I'm not aware of a mechanism where source code mysteriously changes without the presence of cosmic rays, and I'm not aware of any testing mechanisms reliable in the face of cosmic rays. -- c
Re: t/codingstd/perlcritic.t needs to be optional
On Jun 27, 2007, at 11:50 PM, chromatic wrote: I'd like to see an option to run it only on *modified* files. Modified since when? -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: t/codingstd/perlcritic.t needs to be optional
On Wednesday 27 June 2007 22:09:55 Andy Lester wrote: On Jun 27, 2007, at 11:50 PM, chromatic wrote: I'd like to see an option to run it only on *modified* files. Modified since when? Modified since the most recent checkout, of course. Check svn or svk status. Run these time-consuming analysis tests on only the modified files. My theories are: * it sucks a lot less to run the full test suite if it doesn't spend a third of its time verifying that, yes, files that haven't actually changed are still valid * people don't run the full test suite before they commit changes, in part, because it sucks to run the full test suite * if no one touches a valid file, it won't get updated in a new checkout and there's no way previously-valid POD will suddenly become invalid POD There's a reason we don't run a lot of these tests in release tarballs. That reason is that it's exceedingly silly to run those tests in release tarballs. -- c
Re: t/codingstd/perlcritic.t needs to be optional
Modified since when? Modified since the most recent checkout, of course. Check svn or svk status. Run these time-consuming analysis tests on only the modified files. It'd have to be against the last update from svn of the file itself. I'm not sure I like the idea of relying on a given VCS. I know Parrot's hosted in Subversion, but what about the Git folks? It smells funny to me. -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: t/codingstd/perlcritic.t needs to be optional
On Wednesday 27 June 2007 22:38:17 Andy Lester wrote: It'd have to be against the last update from svn of the file itself. Yes. I'm not sure I like the idea of relying on a given VCS. I know Parrot's hosted in Subversion, but what about the Git folks? As soon as they start reporting failures in the metadata tests, I'll start to believe we should consider that they run the full test suite. I haven't seen them report any failures from the metadata tests. Thus I conclude that, if any such folks exist, they don't have a lot of motivation to report failures. It smells funny to me. All I know is that I've made more than my share of commits in the past six months to fix broken tests of non-functional requirements. I'm all for code quality and standards and removing even all warnings, but *people don't run the full test suite reliably before they commit anyway*. Heck, you didn't even *compile* before one of your checkins yesterday. I can't believe that adding more tests--tests that analyze some subset of the 3800 files in the repository and perform a lot of IO to do so--will encourage people to run the tests more often. It's my experience (and advice I give people in exchange for money in professional contexts) that making tests faster and less painful to run encourages people to run them more often. Faster, more frequent feedback enables many very good things. Running all of the coding standards tests on all of the files in the repository--even the ones we didn't change--on every full test run goes against my strongly-held personal advice. We certainly don't do that for the tests of the configuration and code-generation systems, and those are FUNCTIONAL tests. Again, I'm all for code quality. I think these tests are important--but they're only important *if* people run them. Adding minutes to the full test run is one sign of a not-right approach. -- c
