Skip to content

Fix tests that were broken and make importable with Go Modules #1

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

Merged
merged 3 commits into from
Aug 23, 2019
Merged

Fix tests that were broken and make importable with Go Modules #1

merged 3 commits into from
Aug 23, 2019

Conversation

leighmcculloch
Copy link

@leighmcculloch leighmcculloch commented Aug 23, 2019

Summary

Fix up this package so that it can be imported with Go Modules. It is imported by stellar/go as a fork of github.com/throttled/throttled.

Goal and scope

The tests of this package no longer build because changes were made in c99eef3 that remove the store sub-package and related functionality. The tests weren't updated and still reference that deleted package. This has not caused stellar/go any problems because that repo is using Dep to manage dependencies, and Dep ignores all test files. stellar/go is moving to Modules (stellar/go#1634) which is stricter and 💥s when a module contains Go code that is dependent on packages that don't exist.

Summary of changes

  • Remove deprecated_test.go as its counterpart deprecated.go was removed in c99eef3.
  • Remove rate_test.go TestRateLimitUpdateFailures as it tests store functionality that was removed in c99eef3.
  • Change rate_test.go TestRateLimit to use an injected Clock since the way it receives an injected time is through an injected store, and stores were removed in c99eef3.

@leighmcculloch leighmcculloch changed the title Fix tests that were broken in c99eef3 and make importable with Go Modules Fix tests that were broken and make importable with Go Modules Aug 23, 2019
@bartekn bartekn merged commit dfc1b68 into bartekn:master Aug 23, 2019
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.

2 participants