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

Fix integration tests #114

Merged
merged 2 commits into from Sep 12, 2016
Merged

Fix integration tests #114

merged 2 commits into from Sep 12, 2016

Conversation

sambrightman
Copy link
Contributor

  • Always check return value of ntfy_main (no longer using exceptions)
  • Do not use a dict as the default of args.option

The latter accidentally shared state across tests, breaking all
integration tests. However, this was hidden because exceptions are now
caught and integration tests were not checking return values.

* Always check return value of ntfy_main (no longer using exceptions)
* Do not use a dict as the default of args.option

The latter accidentally shared state across tests, breaking all
integration tests. However, this was hidden because exceptions are now
caught and integration tests were not checking return values.
@sambrightman
Copy link
Contributor Author

sambrightman commented Sep 11, 2016

It's not clear to me where the ntfy[xmpp] dependency should go to run the integration test for an optional feature. This failure was previously being hidden as mentioned. CI should pass if .[xmpp] is added to the install lists, but don't think it can go into tests_require? ntfy[xmpp] there would refer to the previously released version. Maybe just get CI passing?

@coveralls
Copy link

Coverage Status

Coverage increased (+7.01%) to 85.115% when pulling b5c8601 on sambrightman:integration_tests into 91c84c3 on dschep:master.

@coveralls
Copy link

coveralls commented Sep 11, 2016

Coverage Status

Coverage increased (+7.01%) to 85.115% when pulling d41f997 on sambrightman:integration_tests into 91c84c3 on dschep:master.

@sambrightman
Copy link
Contributor Author

Pushed the changes to get CI working, not sure if most appropriate.

@coveralls
Copy link

coveralls commented Sep 11, 2016

Coverage Status

Coverage increased (+7.01%) to 85.115% when pulling 62b3537 on sambrightman:integration_tests into 91c84c3 on dschep:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+7.01%) to 85.115% when pulling 62b3537 on sambrightman:integration_tests into 91c84c3 on dschep:master.

@dschep dschep merged commit 438161d into dschep:master Sep 12, 2016
@dschep
Copy link
Owner

dschep commented Sep 12, 2016

Awesome. thanks! That way of handling the xmpp req is fine :)

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

3 participants