ETT-1394 address warning: "CGI::Param called in list context"#234
Conversation
- Include `mb/t/` in `perl-test`
- Fix and tests for `handle_ANALYTICS_REPORT_URL_PI`
…LYTICS_REPORT_URL_PI`
- Not really testable for reasons cited in ticket
- No tests because the method is enormous and the prospect of scaffolding a unit test is daunting.
There was a problem hiding this comment.
The actual fixes look good.
Looking at the tests, it doesn't seem like they'd fail if the changes in the code weren't there. Maybe worth considering if there's any reasonable way to test that there are no warnings, if it's easy (see e.g. https://perlmaven.com/test-for-warnings-in-a-perl-module)? That said, the tests can at least serve as regression tests if nothing else, if we aren't easily able to test the lack of warnings.
…ere are none in the PIFiller tests. - We can probably use this approach when we get around to the Date::Manip warnings, and anything else clogging the logs.
|
@aelkiss This latest addition was a useful one, as noted, when we tackle Date::Manip and other nonsense clogging up the logs. |
- Clean up and fix some outdated comments.
aelkiss
left a comment
aelkiss
left a comment
There was a problem hiding this comment.
This looks good to me. We could potentially refactor some of the warning checking stuff especially if we use it more widely (e.g. when addressing Date::Manip) but this is good for now.
2 participants
Include
mb/t/inperl-testAddress and test
LS::PIFiller::ListSearchResults::handle_ANALYTICS_REPORT_URL_PIAddress and test
MBooks::PIFiller::ListUtils::handle_ANALYTICS_REPORT_URL_PIAddress issue in
MBooks::Operation::LogoutTrap::redirect_and_exitexitand it appears its call toMBooks::View::P_redirect_HTTPalso callsexit.Test::TrapandTest::Exitbut to no avail.Address issue in
PT::Prolog::RunThere are no other issues flagged in the error logs I could find in the past 30 days.