NigelCunningham / pam-MySQL

PAM MySQL
GNU General Public License v2.0
111 stars 61 forks source link

Regression testing would be a good idea. #70

Closed kevgreen-ebay-com closed 3 years ago

kevgreen-ebay-com commented 3 years ago

So, there was a regression in the build process. People noticed. But more people probably noticed the issue that was present before the patch that fixed that and inadvertently caused the regression. shrug.

I have some testing stuff that can help, which I can probably cut & paste into something specific to pam-mysql, but it's designed to test whole servers, so it's using vagrant+virtualbox+ansible+serverspec and that might be a bit much?

@wferi indicates he's got something that proved when the relevant function call dropped out of exports, @NigelCunningham probably has some stuff.

I'd be happy to help, but I guess we need to take stock of what we do have, what the relevant standards are, and so on before trying to implement something.

Thoughts anyone?

PS. Obviously doff our hats to @slimlv to noticing and providing a patch for the build process.

kevgreen-ebay-com commented 3 years ago

First thoughts on a minimum requirement....

Any test harness should probably test:

Then pre-seed some SQL data for test users, and set up a different mysql table + saslauthd profile + pam profile for each different authentication method to allow them to be tested independently.

And for each authentication method:

Also do we need to test for this (and does it apply to all crypt/auth types?):

wferi commented 3 years ago

For a start I think GitHub CI could be employed to at least build-test any pull requests. Meanwhile the Debian package has an autopkgtest, which now tests authentication with a bunch of crypt methods: creates a database, configures PAM for itself and does some authenticate() calls. That's not a unit test, though, but rather an integration test: it tests the installed module.

kevgreen-ebay-com commented 3 years ago

This issue may have been created from the wrong login, and things said by me here with this login are personal and no reflection on the activities or opinions of ECG or Ebay. Disclaimer ends.

NigelCunningham commented 3 years ago

Thanks for the suggestion and apologies for being slow to the party. I just never seem to see the email notifications that I assume Github sends.

Anyway...

Totally independently of this thread, I've been wanting for some time to get some testing into pam-MySQL too. I've recently completed a switch to using Meson as the build system, and have also split the monolith that was pam-mysql.c into a number of separate files. This has made it possible for me to then replace parts of the system (think mysql calls and pam calls) with mocks, so we now have the ability to have unit tests. I've implemented some straight forward, end-to-end tests for a start; more could be added over time. I'll have to take a look at the autopkgtest; perhaps I can make use of some of that too, @wferi?

In the meantime, I've downloaded FreeBSD to check I can get it to compile and work there.

Thanks again!

NigelCunningham commented 3 years ago

I'll close this issue as I've switched build systems and started implementing tests under Meson.