[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #35 from Henrik Krohns --- Sort is just hiding the bug. It should be enough to run a single do_meta_tests/finish_meta_tests in the end, which should iterate all metas over and over until all are ready. I just can't figure out why rule order would matter, even rewrote things now to work in "deterministic" way, firing metas from rule_ready when deps are ready.. and it actually behaves the same way. There's some logical fallacy here that I just can't grasp yet. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735
--- Comment #34 from Sidney Markowitz ---
(In reply to Henrik Krohns from comment #30)
> There's still some strange bug somewhere that makes meta rules behave
> differently on different runs.
The following change in Check.pm makes that go away. I have no idea if this is
a fix or a clue, so I leave it up to you what to do about it:
Index: lib/Mail/SpamAssassin/Plugin/Check.pm
===
--- lib/Mail/SpamAssassin/Plugin/Check.pm (revision 1901358)
+++ lib/Mail/SpamAssassin/Plugin/Check.pm (working copy)
@@ -329,7 +329,7 @@
my %unrun_metas;
RULE:
- foreach my $rulename (keys %$mp) {
+ foreach my $rulename (sort(keys %$mp)) {
my %unrun;
# Meta is not ready if some dependency has not run yet
foreach my $deprule (@{$md->{$rulename}||[]}) {
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #33 from Sidney Markowitz --- (In reply to Henrik Krohns from comment #30) > There should be no > reason for the list to change between runs. > > spamassassin -t -L -D rules-all < message 2>&1 | grep unrun As an easy way to show this, in a trunk directory after a make I ran once ./spamassassin -t -L -D rules-all --siteconfigpath=. < sample-nonspam.txt 2>&1 | grep -o 'dbg: rules-all: unrun.*$' | sort > ~/tmp/onerun and then repeated runs of ./spamassassin -t -L -D rules-all --siteconfigpath=. < sample-nonspam.txt 2>&1 | grep -o 'dbg: rules-all: unrun.*$' | sort | diff -u0 - ~/tmp/onerun show different results every time. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #32 from Sidney Markowitz --- I can't reproduce the sporadic failures in t/basic_meta2.t since the latest fixes to tests, but since it was always sporadic that is not a definitive result. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 Sidney Markowitz changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #9 from Sidney Markowitz --- trunk % svn ci -m "bug 7997 move non-rule settings from 01_test_rules.cf to 01_test_rules.pre" Sendingt/data/01_test_rules.cf Sendingt/data/01_test_rules.pre Transmitting file data ..done Committing transaction... Committed revision 1901358. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 Bill Cole changed: What|Removed |Added CC||billc...@apache.org --- Comment #8 from Bill Cole --- (In reply to Sidney Markowitz from comment #6) > That passed the tests. If you agree with that approach, I'll commit it. +1 -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 --- Comment #7 from Henrik Krohns --- Looks fine to me.. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 --- Comment #6 from Sidney Markowitz --- That passed the tests. If you agree with that approach, I'll commit it. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 --- Comment #5 from Sidney Markowitz --- (In reply to Henrik Krohns from comment #4) > If that's the case, sure, I guess it simplest to allow the few rules in > 01_test_rules.cf remain. Now that I think more about it, I am less sure. I didn't want to make major changes, so I left it the way it was, which is make test runs in the t directory and copies ../rules/*.cf to localrules data/01_test_rules.cf contains rules that tests can assume are defined even if make test is not run from trunk with all trunk/rules make disttest creates a subdirectory, copies MANIFEST files there (which include just a subset of trunk/rules) and runs make and make test from there clear_localrules() can be called by tests that want to make sure that they control what rules are defined. I think that clear_localrules() should clear local rules, even those in 01_test_rules.cf because those are just an arbitrary subset that were being used by tests. We can move these settings to 01_test_rules.pre because they are settings, not rules. What do you think? clear_report_template report _SUMMARY_ clear_headers add_header spam Flag _YESNOCAPS_ add_header all Level _STARS(*)_ add_header all Status "_YESNO_, score=_SCORE_ required=_REQD_ tests=_TESTS_ autolearn=_AUTOLEARN_ version=_VERSION_" ifplugin Mail::SpamAssassin::Plugin::DCC use_dcc 0 endif ifplugin Mail::SpamAssassin::Plugin::Razor2 use_razor2 0 endif ifplugin Mail::SpamAssassin::Plugin::Pyzor use_pyzor 0 endif -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 --- Comment #4 from Henrik Krohns --- (In reply to Sidney Markowitz from comment #3) > > How about adding 01_test_rules.cf to the list of rule files in > clear_local_rules() that it doesn't clear? I haven't quite followed how tests are supposed to work after your changes. So apparently when testing in trunk, ../rules/*.cf is still always copied to localrules? And disttest only pretty much has localrules/01_test_rules.cf? If that's the case, sure, I guess it simplest to allow the few rules in 01_test_rules.cf remain. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 Sidney Markowitz changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #3 from Sidney Markowitz --- t/rule_multiple.t is failing in make disttest, though works in make test In email, hege said about it > I can add the "report _SUMMARY_" to rule_multiple.t, then it works. > Dunno what would be the most clear way to resolve this. Obviously the > report stuff is not "rules" per say, maybe it should even be in > 01_test_rules.pre so it never gets removed. As it says in 01_test_rules.cf # Rules used in the test suite. This allows us to change the # main ruleset without breaking the test suite. How about adding 01_test_rules.cf to the list of rule files in clear_local_rules() that it doesn't clear? -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 Sidney Markowitz changed: What|Removed |Added CC||sid...@sidney.com --- Comment #31 from Sidney Markowitz --- I'm seeing this in sporadic t/basic_meta2.t failures -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 Henrik Krohns changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #30 from Henrik Krohns --- Reopening to investigate and keep 4.0.0 blocked. There's still some strange bug somewhere that makes meta rules behave differently on different runs. You can see this by checking the same message multiple times, and looking at the "unrun dependencies prevented meta XXX from running". There should be no reason for the list to change between runs. spamassassin -t -L -D rules-all < message 2>&1 | grep unrun -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 Henrik Krohns changed: What|Removed |Added CC||apa...@hege.li Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #2 from Henrik Krohns --- This was fixed with Revision 1901346. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997
--- Comment #1 from Sidney Markowitz ---
committed by hege:
revision 1901346
Log:
Test cleanups and fixes.
Note that %patterns has now two exact patterns styles:
- Literal strings match exactly the string. Whitespace is no longer ignored
(any leading and trailing whitelist must match), but consecutive
whitespace is normalized:
q{ FOO } => ''
' FOO ' => ''
- Regular expressions, defined with standard qr// operator:
qr/ FOO / => ''
revision 1901348
Log:
Don't clear any tstprefs() or tstlocalrules() settings with clear_localrules()
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7997] Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997 Sidney Markowitz changed: What|Removed |Added CC||sid...@sidney.com Target Milestone|Undefined |4.0.0 -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7997] New: Test failures indicate problems in tests that need cleanup
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7997
Bug ID: 7997
Summary: Test failures indicate problems in tests that need
cleanup
Product: Spamassassin
Version: SVN Trunk (Latest Devel Version)
Hardware: All
OS: All
Status: NEW
Severity: blocker
Priority: P2
Component: Regression Tests
Assignee: dev@spamassassin.apache.org
Reporter: sid...@sidney.com
Target Milestone: Undefined
Some test failures that only occur when running without the full contents of
trunk/rules have revealed a number of problems and weaknesses in tests. For
example, t/rule_multiple.t contains test patterns
q{ META_BODY_RULE }
q{ META_BODY_RULE_MAX }
Resulting in these actual regex
\s*META_BODY_RULE\s*
\s*META_BODY_RULE_MAX\s*
which both match the string META_BODY_RULE_MAX
All the tests that have similar problems should be fixed even if they are not
currently failing, and SATest.pm should be changed to make it less likely that
tests will be written with these faults.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7992] Capturing and reusing strings for matching across rules
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7992
Henrik Krohns changed:
What|Removed |Added
Resolution|--- |FIXED
Status|REOPENED|RESOLVED
--- Comment #29 from Henrik Krohns ---
Fixed by changing %{FOO} to %\{FOO\} before compile_regexp.
Committed revision 1901347.
--
You are receiving this mail because:
You are the assignee for the bug.
