On 2015-06-22 12:36 PM, Sam Kington wrote: > Hi, > > Delurking, because the latest version of Dancer is causing our unit tests at > $WORK to start creating empty log files inside the current working directory. > This is easily-reproducible: e.g.
Issue created and potential patch attached at https://github.com/PerlDancer/Dancer/issues/1121 I'm poking the rest of the crew to know if we want to just revert or commit that patch. A decision will be forthcoming shortly. Joy, `/anick > >> sam@chimera64:tmp$ cat spamlogfile.pl >> #!/usr/bin/env perl >> >> use Dancer qw(halt); >> sam@chimera64:tmp$ rm -fr logs; perl spamlogfile.pl >> sam@chimera64:tmp$ ls -lah logs/ >> total 40K >> drwxr-xr-x 2 sam sam 4.0K Jun 22 17:01 . >> drwxrwxrwt 10 root root 36K Jun 22 17:01 .. >> -rw-r--r-- 1 sam sam 0 Jun 22 17:01 development.log > > A workaround is to say use Dancer qw(:syntax), but there are two problems > with that: > > (1) this is a change in behaviour that wasn’t documented as such, and it > requires all auxiliary non-server Dancer code to be changed; and > > (2) passing :syntax imports all sorts of Dancer symbols that can potentially > clash with a parent class. > > I’ve spent the afternoon debugging point 2. Our end-to-end tests run under > Test::Dancer, and we have a separate class that tests individual method calls > as if they were Dancer requests (including setting the status code in case of > errors). It previously said use Dancer () and explicitly called > Dancer::status - and under Dancer 1.3138, running unit tests using this class > would generate spurious empty log files. > > I patched that to say use Dancer qw(:syntax) - and now the code fails because > the parent class implements methods true, false and redirect, which the > method call class never gets to call because it’s had Dancer’s symbols stuck > in its own package instead. So this now needs to say use Dancer qw(:syntax > !true !false !redirect), and in practice I needed to write an automated test > script that checked that all of a parent class’s methods were explicitly not > imported from Dancer, because if I change anything upstream I need to change > the import statement in the child classes. > > The problem appears to have been introduced in > https://github.com/PerlDancer/Dancer/commit/e9e4e087528c2734dd6e9c922105eac3cd2e53e7 > > The actual problem is further down in Dancer::Config - line 227 in 1.3138 - > where the settings are read. $SETTINGS at this point contains default values, > and part of setting confdir involves creating a logs subdirectory. Previously > the code wouldn’t get this far. > > Presumably a workaround would be to not create a log directory immediately, > but then if you were running a real Dancer server you’d want to know as soon > as possible that your log directory wasn’t writable, so that’s not going to > be acceptable to many people. > > Alternatively, any code that says e.g. use Dancer qw(halt status) should be > seen as clearly not wanting to actually start a Dancer server or anything > like that. That sounds like a scary change to make, though. > > Or you could defer parsing of the Dancer config until something actually > happens. That’s also vague and terrifying. > > Or just revert that particular commit / PR. I don’t fully understand why it > does what it does, though. If there’s a way of cleanly telling apart (1) a > hash of settings that was explicitly passed to Dancer from (2) the default > settings that, chances are, you didn’t intend to use, that would be the best > solution to my mind. > > Sam > _______________________________________________ dancer-users mailing list [email protected] http://lists.preshweb.co.uk/mailman/listinfo/dancer-users
