Leonidas-from-XIV / slacko

A neat interface for Slack
https://leonidas-from-xiv.github.io/slacko
Other
81 stars 11 forks source link

Functional tests #14

Closed jerith closed 7 years ago

jerith commented 7 years ago

This is the start of my attempt at building a suite of functional tests for slacko. It's nowhere near ready to merge, but it's a reasonable starting point for a conversation about what we'd like this to become.

The only change to the slacko code itself is that base_url is now a reference and a new set_base_url function (which I've told ocamldoc to leave out) allows it to be set externally.

I haven't quite figured out how to handle the tension between strict test cases that make use of known data and the much more relaxed tests that can be run against the real API to make sure slacko continues to work properly as the slack API evolves over time.

For now, I plan to add simple tests around a few more functions ~and maybe figure out how to generate test coverage reports~ (coverage is done).

Here are some problems the tests have already uncovered:

I'd prefer to address these problems in separate branches and keep this one focused on the tests themselves, but it would probably make sense to merge or rebase some of the fixes back into this branch.

jerith commented 7 years ago

The coverage thing turned out to be pretty easy. I enabled coveralls for my fork (see https://coveralls.io/github/jerith/slacko for the results) but it'll have to be enabled separately for the main repo.

Adding test runs to the builds seems to make them significantly slower. :-(

jerith commented 7 years ago

These tests would be much easier to work with if there were a good way of turning errors back into strings (which there is already an issue for: #4).

I'm starting to uncover some bugs with these tests. I don't really want to fix them in this branch, because the focus here is on writing tests for the existing code, so I'll make a note of them in the PR description above (where they'll be more visible than buried in the comments) so they can be addressed separately.

jerith commented 7 years ago

How about this: let's get this into a "releaseable" shape, since the last release is very old and publish it. Then iterate on writing tests and fixing what is broken.

That's pretty much where I was headed too. I'd like to at least cover the issues from #10 before merging, but that shouldn't take too much work.

jerith commented 7 years ago

18, #19, and #20 fix all the problems that currently require tests to be skipped in this branch.

Is there anything else you'd particularly like tests for in this branch? chat_post_message is the only thing I think really needs to be tested before a release, but I'm happy to do that in a new PR after this one is merged.