notqmail icon indicating copy to clipboard operation
notqmail copied to clipboard

Test qmail-inject recipient address qualification.

Open schmonz opened this issue 3 years ago • 5 comments

Using qmail-qfilter (proposed in #227), turn the minimal reproducer from #147 into an automated regression test. It's an integration test, not a unit test, because it executes qmail-inject[*] the same way our systems would. Thanks to qmail-qfilter, the test does not involve a real queue in any way.

#229 probably fixes #147, and these tests will help us be confident of that.

[*] Or rather, tests/testable/qmail-inject, which is identical modulo linking with a custom definition of auto_qmail that points into the source tree

schmonz avatar Jan 20 '22 14:01 schmonz

Almost there ;)

I would very much prefer to not have to create a separate inject binary. Would it be possible to let the filter grep for the expected "To: " line and return success or failure? Then it should be possible to just qmail-inject and look at it's return code to see if the filter has accepted the mail or not. At least that would be the solution with the least possible side effects I can think of.

DerDakon avatar Jan 21 '22 15:01 DerDakon

Yes, I'd feel much better if we could directly test the real qmail-inject we're about to install. Trouble is, before we've installed, qmail-inject hits die_chdir() -- and if we have installed notqmail already, /var/qmail is still the wrong place to be looking. So at least in this one case so far, we need a way to override the value of auto_qmail for testing.

But how? Linker tricks aren't portable enough. Relinking a fake auto_qmail into the real qmail-inject is a no-no (people need to be able to run the tests and then install). Maybe we introduce an environment variable for the purpose of overriding auto_qmail, being careful to name it something strange enough that no admin would happen to already have it set? Other options?

schmonz avatar Jan 21 '22 22:01 schmonz

I wonder why all tests pass if this should detect the regression?

DerDakon avatar Jan 22 '22 17:01 DerDakon

I've made the failing test fail the build, so the checks should look uniformly bad now :-)

In an attempt to avoid

  • hand-duplicating an entire rule just so I can hand-change a tiny part of it
  • relying on non-portable linker tricks
  • introducing a test-motivated environment variable (and function lookup) into production code
  • using chroot or another workaround that would require running tests as root

... tests can depend on testable. integrationtest_inject depends on it, causing the top-level build to be copied into a subdirectory of tests where it can be messed with in unsurprising ways.

Copying seems wasteful, but for our autobuilds the time consumed is in the noise, and for local development we don't need to be running the integrationtest target most of the time, just the unittest target.

This approach seems weird but also obvious enough that we'll be unlikely to screw things up.

schmonz avatar Jan 22 '22 22:01 schmonz

Also, the filter no longer drops a sentinel-file turd to indicate the bug. Instead it uses the customerror API from #195. Perfect fit :-)

schmonz avatar Jan 22 '22 22:01 schmonz