You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@spamassassin.apache.org on 2022/05/01 04:15:19 UTC

[Bug 7982] New: make test failures running from 4.0.0 install package

https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

            Bug ID: 7982
           Summary: make test failures running from 4.0.0 install package
           Product: Spamassassin
           Version: 4.0.0
          Hardware: PC
                OS: Mac OS X
            Status: NEW
          Severity: blocker
          Priority: P2
         Component: Building & Packaging
          Assignee: dev@spamassassin.apache.org
          Reporter: sidney@sidney.com
  Target Milestone: Undefined

Steps to reproduce:

Untar the 4.0.0rc1 tarbhall into a directory and cd into it

perl Makefile.PL < /dev/null
make
make test

Many tests fail that do not fail if you do the same thing in a svn checkout of
trunk or of the spamassassin_release_4_0_0_rc_1 tag.

A number of the failures are the results of 20_aux_tlds.cf being removed from
MANIFEST in revision 1880742 which also removed 60_bayes_stopwords.cf

When I copy the two rules files into the rules directory most but not all of
the test failures are fixed. When I copy the rest of the rules installed by
sa-update into the rules directory, more of the failing rules are fixed. That
implies that some failing tests need rules that were never in MANIFEST.

Even after copying all files from
/var/lib/spamassassin/4.000000/updates_spamassassin_org/*.cf to rules/ there
are still a number of test failures that don't happen running make test from an
svn checkout.

I think this means that 1) we have to work out how we want make test to work
for a distribution tarball, specifically how that can interact with sa-update
or if we want to allow someone to run make test before they run make install
and sa-update; and 2) look at and fix whatever the problems are with the
remaining tests that fail for other reasons,

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #4 from Henrik Krohns <ap...@hege.li> ---
Yeah it seems lots of files like 10_default_prefs.cf is required too.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #2 from Henrik Krohns <ap...@hege.li> ---
For 60_bayes_stopwords.cf there is no need. Bayes.pm has a hardcoded stopword
list in case no cf is found. I don't think it would even have any effect on
tests.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #8 from Kevin A. McGrail <km...@apache.org> ---
What's the behavior of a 3.4.6 tar ball and make test?  What's in that
MANIFEST?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #17 from Sidney Markowitz <si...@sidney.com> ---
Answering in reverse order:
(In reply to Henrik Krohns from comment #16)
> we should stop SATest.pm from copying any cf from trunk/rules at all

That's something I was already considering while working on this. It might
still be reasonable to make it easy to test against the current rules instead
of the packaged subset, but that would be better as an explicit option rather
than something that happens when you don't pay attention to the test
environment.

(In reply to Henrik Krohns from comment #15)
> Does the fix make sense though? As mentioned, isn't it a advantage that "make
> test" actually tests that current trunk/rules does what it's supposed to
> correctly?

I have mixed feelings about this. I think that tests should be as
self-contained as possible. If they depend on current rules, they can break
when rules change. They can even break silently by not failing when they
should. The advantages of 01_test_rules.cf are 1) It documents the smallest set
of rule dependencies that are needed for the tests; and 2) It encourages one to
write tests so that they don't depend on rules, because any new dependencies
will require adding something to 01_test_rules.cf.  The disadvantages are that
it freezes those rules as used in the tests.

One thing about doing it this way, is that running make test from a trunk
checkout directory will see all the current rules from the rules directory.
Running it from an install package directory will have the stripped down rules
directory and will test the more self-contained frozen version in
01_test_rules.cf. So tests can be run both ways, which should detect different
problems.

I don't have any concern that users will be confused by the files, they'll
never see it. I do feel off about the distribution and tests depending on an
arbitrary snapshot of the entire set of rules as they are when the release is
tagged. Using only a small subset of rules in 01_test_rules.cf is more
controlled and easier to keep track of.

Along those lines, I would really like it if someone could look at
sql_based_welcomelist.t and figure out how to make it dependent on much fewer
rules. As far as I can tell it has test cases that are designed to trigger
different rules and it checks that the expected results get processed. What it
is testing has nothing to do with those specific rules, they are just used to
see if the expected strings are generated. But each such rule adds a
requirement for one or more rule files to be available.

Anyway, what I want to do is continue seeing what has to go into
01_test_rules.cf to allow the tests to succeed. Once I do that, or hit a
roadblock, we can compare that to the option of copying all the rules into the
install. I already have a patch that does that, so we can decide then.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #13 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Kevin A. McGrail from comment #12)
> Primarily, I would likely look at what changed between 3.4.6 and 4.0

I can do that. I do think it would be much simpler all around if the tests did
not require the rules that happen to be in trunk on the day the tests were
written. That's just too brittle, tests should be more self-contained and
written to check specific functionality. sql_based_welcomelist.t and
sql_based_whitelist.t are particularly bad, judging from how many rules they
seem to require.

That's a good point to see if I can understand what exactly is different in
3.4.6 that makes this not a problem there.

I do have a small patch to the build script that adds the rules to MANIFEST in
the working directory before the tar is built and immediately reverts it so we
would not have to keep MANIFEST in svn updated with rule changes. But as you
imply, better to understand and fix the problem instead of taking a
sledgehammer to it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #15 from Henrik Krohns <ap...@hege.li> ---
(In reply to Sidney Markowitz from comment #14)
> Ok, the real problem seems to be frustratingly simple.
> 
> t/SATest.pm uses t/data/01_test_rules.cf for its rules so it does not have
> to depend on the ever changing contents of rules. When running in a trunk
> checkout directory, all the current rules in the rules directory are
> available, so nobody noticed that the file has gotten out of date with
> changes to the tests.
> 
> As far as I can tell the correct way to fix this is to identify the rules
> dependencies of the tests and update 01_test_rules.cf to suit. I'll start
> working on that.

Does the fix make sense though? As mentioned, isn't it a advatage that "make
test" actually tests that current trunk/rules does what it's supposed to
correctly?

If you have stuff in 01_test_rules.cf, it's overridden anyway from trunk/rules
as all the stuff is copied to t/.../localrules.

This means now you would have to always remember to check if 01_test_rules.cf
is up to date and nothing in trunk/rules is overriding it. This seems to make
01_test_rules.cf obsolete if you ask me.

It would be trivial to just copy trunk/rules to distribution tarball, I don't
understand why it would even confuse users, they probably wouldn't even notice
it..

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #18 from Henrik Krohns <ap...@hege.li> ---
(In reply to Sidney Markowitz from comment #17)
> Along those lines, I would really like it if someone could look at
> sql_based_welcomelist.t and figure out how to make it dependent on much
> fewer rules. As far as I can tell it has test cases that are designed to
> trigger different rules and it checks that the expected results get
> processed. What it is testing has nothing to do with those specific rules,
> they are just used to see if the expected strings are generated. But each
> such rule adds a requirement for one or more rule files to be available.

I can fix it easily, but it depends on how you want to fix it.

For example it's assumed that data/nice/002 hits these rules to get a -2 score,
which the string match expects

-1.0 MAILING_LIST_MULTI     Multiple indicators imply a widely-seen list
                            manager
-1.0 NICE_REPLY_A           Looks like a legit reply (A)

Even if I fix sql_based_welcomelist.t, can we be sure that no other tests rely
on a nice mail getting negative score? It could get convoluted.

Then we must decide if we want to make sql_based_whitelist.t completely
independent of 01_test_rules.cf. Why should it rely on that if we can just put
all the needed rules into sql_based_whitelist.t directly, that would be the
most well documented and maintained option? This can simply be achieved by
cleaning  out $localrules from sql_based_whitelist.t.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #22 from Sidney Markowitz <si...@sidney.com> ---
The test t/spf.t calls disable_compat "welcomelist_blocklist" and then expects
to see USER_IN_SPF_WHITELIST in the results. Is that a correct thing to have in
the test now, or should it be changed to look for USER_SPF_WELCOMELIST?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

Kevin A. McGrail <km...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@apache.org

--- Comment #6 from Kevin A. McGrail <km...@apache.org> ---
I like the idea of just including the relevant rules dirs for the purpose of
make test but not installing them.

NOTE: i seem to remember part of the reason we added a few rules to the
MANIFEST was ruleqa related.  Don't make it a blocker but when we do go to 4.0,
the entire ruleqa/masscheck system needs to be rejiggered for 4.0.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #27 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Henrik Krohns from comment #26)
> Ok there's now a function to clear out any external rules.

It needed the add_header all Status line that is in 01_test_rules.cf, but after
that one addition it appears to work fine.

I almost have all the tests set up now.

Thanks

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #12 from Kevin A. McGrail <km...@apache.org> ---
Sidney, 

Primarily, I would likely look at what changed between 3.4.6 and 4.0 since
those don't look like new tests to see why they now fail.  I would double
confirm that 3.4.6 passes a make test first.  Then see with 4.0.  Then compare
the MANIFESTS and manually add a few files.

Let me know if you want some help with that.

Re: adding more files, the MANIFEST file drives the build process so you would
have to change the build process to add more files.  It might ripple from there
and some of the things like the make disttest might throw errors as well that
could be documented in the build/README process.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #28 from Sidney Markowitz <si...@sidney.com> ---
All tests pass running make test in a directory of an extracted release tar.

I pared down the contents of t/01_test_rukles.cf and put some local rule and
configuration options in some tests to make them more self-contained. I added
20_aux_tlds.cf to MANIFEST but that's the only rule file that needed to be
added to the install package. Even if the tlds change in trunk, this will be a
good enough enough snapshot that allows tests to run.

I ran all optional tests that I could, including ssl and root. I skipped ldap,
ipv6, and the SQL tests other SQLite because I couldn't or wouldn't set them
up. I also skipped rule name tests, pl, and sawampersand after reading the
comments in them that basically say not to bother.

$ svn ci -m "bug 7982 fix tests failing when run from release tarball, removing
dependency on rules that are in trunk"

Transmitting file data .................................done
Committing transaction...
Committed revision 1900547.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #23 from Henrik Krohns <ap...@hege.li> ---
(In reply to Sidney Markowitz from comment #22)
> The test t/spf.t calls disable_compat "welcomelist_blocklist" and then
> expects to see USER_IN_SPF_WHITELIST in the results. Is that a correct thing
> to have in the test now, or should it be changed to look for
> USER_SPF_WELCOMELIST?

I would expect all the original rules like t/spf.t and t/*whitelist*.t to hit
WHITELIST rules.

All the new t/spf_welcome_block.t etc are supposed to hit WELCOMELIST rules.

I assumed that after some time testing old names would become deprecated and
renamed over like t/spf_welcome_block.t -> t/spf.t, so we only test for
WELCOMELIST. If required, that could probably even be done now, most likely the
WLBL stuff is tested enough already and we should not changes to trunk that
breaks it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #25 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Henrik Krohns from comment #24)
> I'll see if I can isolate it today.

That's great. What I'm realizing more as I work on this is that there should be
more use of putting test-specific things in tstlocalrules or tstprefs in the
test where we have been putting such things in t/data/01_test_rules.cf.
Anything which is only used by one or two tests can be moved into the test. I'm
going to evaluate everything that I found needed to be added to
t/data/01_test_rules.cf to get tests to pass on that basis.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #3 from Sidney Markowitz <si...@sidney.com> ---
Following up some questions and suggestions from Kevin on the dev list:

Copying 20_aux_tlds.cf to the rules directory fixes some but not all of the
failures.

Some failures I got were the result of running the tests in s nested
subdirectory that caused the socket path of spamd to be longer than the Linux
maximum of 108 characters. Since the error message is clear enough to let
someone know they have to move the installation directory up a bit, I don't
think this has to be fixed in any way.

More failures were fixed by copying the remaining *.cf files from sa-update or
trunk into the rules directory. I'll start on a binary search to narrow down
exactly which files are necessary, and maybe they can be added to MANIFEST if
that's the easiest solution. However there were still some failures after that.

The remainder of the failures were fixed by adding symlinks to the rulesrc and
t.rules directories in trunk. I think this is something we have to fix, because
people should expect to run make test on an installation tarball without a
separate svn checkouts.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #5 from Henrik Krohns <ap...@hege.li> ---
Basically the tests have went too far in the "test that my trunk/rules commit
works", instead of being self-sufficient tests for testing internal functions.

Like for example now many tests assume to find USER_IN_WHITELIST and
USER_IN_WELCOMELIST rules and test that our WLBL changes work. The tested rules
really should be defined in *.t files and not in trunk/rules.

But I also see how using trunk/rules makes development, testing and even
automatic update releasing easier.

It would be simpler to just leave a full copy of trunk/rules to the
distribution tarball. As I already pointed out, none of those are copied to
system in "make install", so there are no consequences. Maybe clearly document
it somewhere that they are for the purposes of "make test".

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #11 from Sidney Markowitz <si...@sidney.com> ---
I identified 12 tests that require a total of 20 rules/*.cf files.
That does not count tests that I didn't run, including network and root tests
and any others that got skipped in my configuration.

I encountered a huge problem when I tried adding just those required files to
the rules directory: t/basic_meta.t started failing because some of those rules
have dependencies on others that were not in the rules directory.

I think the only possible solutions are to either go through all those tests
and change them to test without requiring rules from the rules directory, or
else go ahead and include all the rules in the installation tar.

I don't think it will be practical to have to update MANIFEST every time a rule
file is added, removed or renamed. I don't like the prospect of manually
putting all rule files in MANIFEST as part of the release process. Since the
release build script already gathers all the rule files to make the rules tar
that is distributed for running a local sa-update, the script could put the
files in the install tar at the same time.

Is there a problem if we distribute the install tar with files that are not
listed in MANIFEST? Is there anything that checks for extra files?

Any thoughts or other suggestions on this?

In case anyone will want to work on removing the dependency of tests on rules,
here is the list of what I found. This does not include the large number of
dependencies on 20_aux_tlds.cf

basic_meta2.t requires 10_default_prefs.cf

header_utf8.t requires 60_welcomelist.cf

mimeheader.t requires 10_default_prefs.cf

priorities.t requires 23_bayes.cf 60_welcomelist.cf

priorities_welcome_block.t requires 23_bayes.cf 60_welcomelist.cf

shortcircuit.t requires 60_shortcircuit.cf

welcomelist_addrs.t requires 60_awl.cf

whitelist_addrs.t requires 60_awl.cf

welcomelist_subject.t requires 60_welcomelist_subject.cf

whitelist_subject.t requires 60_welcomelist_subject.cf

t/sql_based_welcomelist.t requires 10_default_prefs.cf 10_hasbase.cf
20_body_tests.cf 20_compensate.cf 20_drugs.cf 20_freemail.cf
20_freemail_domains.cf 20_head_tests.cf 20_html_tests.cf 20_meta_tests.cf
20_ratware.cf 20_uri_tests.cf 20_vbounce.cf 50_scores.cf 60_awl.cf

t/sql_based_whitelist.t requires 10_default_prefs.cf 10_hasbase.cf
20_body_tests.cf 20_compensate.cf 20_drugs.cf 20_freemail.cf
20_freemail_domains.cf 20_head_tests.cf 20_html_tests.cf 20_meta_tests.cf
20_ratware.cf 20_uri_tests.cf 20_vbounce.cf 50_scores.cf 60_awl.cf

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #7 from Sidney Markowitz <si...@sidney.com> ---
Are the files in t.rules used by make test? If that's the case then why not
include them in MANIFEST just like the files in the t directory?

I deleted the rulesrc symlink and that seems not to be needed by any tests.

Since the script to build the distribution creates the rules tar file for local
sa-update, we can modify the script to also put the same files in a rules
directory on the install tar. That seems easier than always modifying MANIFEST
for something that changes as often as the rules just to capture the contents
for a make test. That and adding t.rules files to MANIFEST should be all that
is needed.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #9 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Kevin A. McGrail from comment #8)
> What's the behavior of a 3.4.6 tar ball and make test?  What's in that
> MANIFEST?

3.4.6 just works. The only .cf file it has in it's MANIFEST that trunk's
doesn't is 20_aux_tlds.cf. I suspect that the failures in trunk are new tests
that assume the existence of other rules files and which were never tested
outside of a trunk checkout directory that has all the rules,

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #14 from Sidney Markowitz <si...@sidney.com> ---
Ok, the real problem seems to be frustratingly simple.

t/SATest.pm uses t/data/01_test_rules.cf for its rules so it does not have to
depend on the ever changing contents of rules. When running in a trunk checkout
directory, all the current rules in the rules directory are available, so
nobody noticed that the file has gotten out of date with changes to the tests.

As far as I can tell the correct way to fix this is to identify the rules
dependencies of the tests and update 01_test_rules.cf to suit. I'll start
working on that.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #21 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Sidney Markowitz from comment #19) 
> I don't think it is worthwhile to rewrite sql_based_welcomelist.t. As you
> say, other rules could do the same thing with data/nice/002.

Now that I have slept on it, I think it is totally a correct thing to do to
rewrite sql_based_welcomelist.t, but not to give it internal rules.

The biggest change in the test compared to what it did in 3.4.6 is that it
checks for an exact score result. The subtests look for scores 0, -2, -4, -6,
-8, 9.837 and 0

When I run with a minimal rule set and just the changes to 01_test_rules.cf
that I added to get the other tests to pass, the scores that the subtests get
are 0, 0, 0, 0, 0, 10, and 0

I don't know if those new 0's mean that the test is not getting testing what it
is supposed to.

Henrik, can you come up with a rewrite of the test that still tests what it
needs to in the sql based welcomelist, but does not break if the scores come
out somewhat different? That should solve the biggest problem here.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #24 from Henrik Krohns <ap...@hege.li> ---
(In reply to Sidney Markowitz from comment #21)
> The biggest change in the test compared to what it did in 3.4.6 is that it
> checks for an exact score result. The subtests look for scores 0, -2, -4,
> -6, -8, 9.837 and 0
> 
> When I run with a minimal rule set and just the changes to 01_test_rules.cf
> that I added to get the other tests to pass, the scores that the subtests
> get are 0, 0, 0, 0, 0, 10, and 0
> 
> I don't know if those new 0's mean that the test is not getting testing what
> it is supposed to.
> 
> Henrik, can you come up with a rewrite of the test that still tests what it
> needs to in the sql based welcomelist, but does not break if the scores come
> out somewhat different? That should solve the biggest problem here.

It's supposed to check the AWL database keeps track of the total score as it
should.

I'll see if I can isolate it today.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #16 from Henrik Krohns <ap...@hege.li> ---
And if you/we want to go with the 01_test_rules.cf route, when we should stop
SATest.pm from copying any cf from trunk/rules at all. Then is it's 100% clear
that tests only use something from 01_test_rules.cf

  for $tainted (<../rules/*.cf>) {
    $tainted =~ /(.*)/;
    my $file = $1;
    $base = basename $file;
    copy ($file, "$localrules/$base")
      or warn "cannot copy $file to $localrules/$base: $!";
  }

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #19 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Henrik Krohns from comment #18)
> For example it's assumed that data/nice/002 hits these rules to get a -2
> score, which the string match expects
> 
> -1.0 MAILING_LIST_MULTI     Multiple indicators imply a widely-seen list
>                             manager
> -1.0 NICE_REPLY_A           Looks like a legit reply (A)

That explains why it is such a problem. If the rule is written like that we
can't even consider leaving it relying on the rules in trunk. The test in 3.4.6
does not check for any exact score. How can we have a test that relies on the
current live rule set always producing a score of exactly -2 on that sample
email? And that score has nothing to do with what this test is supposed to be
testing, who cares if it is exactly -2?

The NICE_REPLY_A rule isn't even mentioned in the test itself. I can see from
the log of a successful run of the test that those two rules appear in the
headers. But that is not true if I don't have the trunk version of 72_active.cf
in rules. Without that version of 72_active the test fails and there is no
indication as to how it expected to get a -2 score, only that it didn't.

I don't think it is worthwhile to rewrite sql_based_welcomelist.t. As you say,
other rules could do the same thing with data/nice/002. Now that I know what is
going on, I can look in the logs for a successful run of a test to see exactly
which rules end up used, and I can add those to 01_test_rules.cf. That is a lot
more sensible than if I had to include all of all 16 rule files that the test
needs and pare it down with trial and error.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #10 from Sidney Markowitz <si...@sidney.com> ---
Now I tried it without the link to t.rules and it still passed. I guess more of
the failures had to do with the too long path than I had recognized.

Next step, I'll binary search through the files in rules to see how few are
actually needed. Perhaps this fix doesn't need to be much more than restoring
20_aux_tlds.cf to MANIFEST and maybe just a little bit more.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #20 from Kevin A. McGrail <km...@apache.org> ---
One POV to consider: I view the code and the rules as separate releases. 
Making sure the code can run with the rules is the goal, not testing the rules.

So having the minimal rules/config/etc. in the tar to prove the code works
would be my goal.  If it's simple to copy all the rules, etc. to get past this
blocker, I'm neutral on that.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |apache@hege.li

--- Comment #1 from Henrik Krohns <ap...@hege.li> ---
I believe it was just a mistake to remove 20_aux_tlds.cf from MANIFEST in
Revision 1880742.

You can add it back. It is not installed with "make install" etc, if you check
Makefile.PL, all the local.cf and *.pre files are manually defined there. So
having rules/20_aux_tlds.cf in distribution has no consequences.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

--- Comment #26 from Henrik Krohns <ap...@hege.li> ---
Ok there's now a function to clear out any external rules.

sql_based*.t are completely self sufficient. Except for 20_aux_tlds.cf to not
complain about missing tlds..

Sending        t/SATest.pm
Sending        t/sql_based_welcomelist.t
Sending        t/sql_based_whitelist.t
Transmitting file data ...done
Committing transaction...
Committed revision 1900512.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7982] make test failures running from 4.0.0 install package

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7982

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|Undefined                   |4.0.0
                 CC|                            |sidney@sidney.com
                 OS|Mac OS X                    |All
           Hardware|PC                          |All

-- 
You are receiving this mail because:
You are the assignee for the bug.