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

Support unit testing Perform funcs which push jobs #52

Closed
wants to merge 1 commit into from

Conversation

mperham
Copy link
Contributor

@mperham mperham commented Jan 7, 2021

See #51 for details. WDYT?

@bploetz
Copy link

bploetz commented Jan 7, 2021

@mperham I'll have to pull this down and try it for real, but this looks like it solves the problem nicely! 👍

@mperham
Copy link
Contributor Author

mperham commented Jan 7, 2021

If you can think of a clean way to trigger Push() errors, that might be a nice addition to this better testing support.

@bploetz
Copy link

bploetz commented Jan 7, 2021

@mperham could you simply add an additional argument to the ExecuteWithResult function:

func (tp *testExecutor) ExecuteWithResult(specjob *faktory.Job, p Perform, simulatePushError bool) (ExecuteResult, error) 

Then pass the simulatePushError flag down to the internal methods accordingly, and if it's set to true have Push return an error.

@bploetz
Copy link

bploetz commented Oct 28, 2022

@mperham do you have any plans to release something like this to support testing jobs that enqueue other jobs? Since you suggested using helper.With() to push jobs within jobs, I'm now back to this original problem where I can't test doing it that way.

@mperham
Copy link
Contributor Author

mperham commented Oct 28, 2022

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.

@bploetz
Copy link

bploetz commented Oct 28, 2022

Isn't this PR an initial spike?

@mperham
Copy link
Contributor Author

mperham commented Oct 28, 2022 via email

}

func (h *testHelper) TrackProgress(percent int, desc string, reserveUntil *time.Time) error {
return nil
Copy link
Contributor Author

@mperham mperham Oct 28, 2022

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?

@mperham
Copy link
Contributor Author

mperham commented Oct 28, 2022

Wait, I created this PR??? 😆 I'm very confused.

@mperham
Copy link
Contributor Author

mperham commented Nov 7, 2022

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.

@mperham mperham closed this Nov 7, 2022
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