Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement API connection test via Test-SlackApi #54

Merged
merged 6 commits into from
Feb 19, 2018
Merged

Implement API connection test via Test-SlackApi #54

merged 6 commits into from
Feb 19, 2018

Conversation

hmmwhatsthisdo
Copy link
Contributor

@hmmwhatsthisdo hmmwhatsthisdo commented Feb 17, 2018

Resolves #1.

Once #53 is merged in, augmenting this to take advantage of the additional error output should be very straightforward.

I'm not sure if we want to write a Pester test for checking the API's availability from inside of AppVeyor (it would cause builds to fail if the Slack API is having issues), but it's at least a thought.

@RamblingCookieMonster
Copy link
Owner

Looks good to me, apart from a silly nitpick: Would you mind moving the help into the function body?

Tests against Slack from AppVeyor make sense to me; would probably just want to be judicious with their use - with the rate limiting you added, I suppose we should be good : )

We could certainly serialize expected results/errors/etc, but having tests that hit the actual API might catch breaking changes to the API - if there are Slack issues during a test, I could always just re-run the build

Cheers!

@hmmwhatsthisdo
Copy link
Contributor Author

hmmwhatsthisdo commented Feb 18, 2018

Would you mind moving the help into the function body?

Whoops! Didn't notice I had put it outside the body on that one. It's been fixed.

Now that API error handling has been merged in, I should be able to apply it here without much trouble. I should have something pushed within the next hour or two.

@hmmwhatsthisdo
Copy link
Contributor Author

hmmwhatsthisdo commented Feb 18, 2018

API error handling works, but while testing things out I realized there's a minor bug with the error-handler code in Send-SlackAPI: failures that occur below the API level (i.e. slack.com being un-resolvable due to not having Internet access) will throw an exception when trying to parse the (non-existent) error message. I'll make an issue/PR for this shortly. See #55/#56

@RamblingCookieMonster
Copy link
Owner

Awesome, thanks! Merging

@RamblingCookieMonster RamblingCookieMonster merged commit eddf2ae into RamblingCookieMonster:master Feb 19, 2018
@hmmwhatsthisdo hmmwhatsthisdo deleted the Test-SlackApi branch February 19, 2018 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants