-
Notifications
You must be signed in to change notification settings - Fork 43
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
Support unit testing Perform funcs which push jobs #52
Conversation
@mperham I'll have to pull this down and try it for real, but this looks like it solves the problem nicely! 👍 |
If you can think of a clean way to trigger Push() errors, that might be a nice addition to this better testing support. |
@mperham could you simply add an additional argument to the
Then pass the |
@mperham do you have any plans to release something like this to support testing jobs that enqueue other jobs? Since you suggested using |
I add features based on demand these days. If no one asks for it, I don't add it. Send a PR if you have an initial spike we can collaborate on. |
Isn't this PR an initial spike? |
Sorry, didn’t notice the context, just replying on my phone.
…On Fri, Oct 28, 2022, at 11:15, Brian Ploetz wrote:
Isn't this PR an initial spike?
—
Reply to this email directly, view it on GitHub <#52 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAWX54EWFTCH2E5BP5NEDWFQJ4TANCNFSM4VZQ2CRQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
} | ||
|
||
func (h *testHelper) TrackProgress(percent int, desc string, reserveUntil *time.Time) error { | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating a separate type which doesn't implement much of the interface, maybe use the standard jobHelper type with a swappable Push() fn?
Wait, I created this PR??? 😆 I'm very confused. |
Unfortunately I'm not going to be able to get to this for a while. If you want to create a new PR which solves your issues, I'd be happy to review. |
See #51 for details. WDYT?