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

Inline mode for tests #290

Closed
sazor opened this issue Mar 9, 2020 · 15 comments
Closed

Inline mode for tests #290

sazor opened this issue Mar 9, 2020 · 15 comments

Comments

@sazor
Copy link

sazor commented Mar 9, 2020

Hello, is there any plans to implement inline mode for tests (as in Sidekiq) to execute job immediately?

@mperham
Copy link
Collaborator

mperham commented Mar 9, 2020

I have no plans to implement it myself; I see inline tests as a smell, like overly large factories. But a simpler block helper where you could do something like this might be welcome:

def test_something
  Faktory.inline do
    call_something_which_creates_jobs
  end
end

@sazor
Copy link
Author

sazor commented Mar 9, 2020

Got it. Thank you!
Sure, it isn't a problem to implement it on top of the client. Just wanted to check if it might be a part of the library.
P.S. I was talking about Golang client. So I'm not sure if it's possible to implement helper like this.

@mperham
Copy link
Collaborator

mperham commented Mar 9, 2020

Agreed, I don't think Go is flexible enough to do this type of thing.

@mperham mperham closed this as completed Mar 9, 2020
@adsteel
Copy link

adsteel commented Jan 11, 2024

@mperham what would you recommend for an integration testing pattern, specifically with the Go implementation?

@mperham
Copy link
Collaborator

mperham commented Jan 11, 2024

You should test your job funcs but integration testing the entire end-to-end (starting up FWG, processing a queue, shutting down and asserting results) is not a use case I designed for. Is that the scenario you are asking about?

@adsteel
Copy link

adsteel commented Jan 11, 2024

Something like that would work, yes. In Sidekiq I used inline extensively and that seemed to provide the deploy guarantees we needed. Now I'm looking for patterns that provide similar guarantees in Faktory.

@mperham
Copy link
Collaborator

mperham commented Jan 11, 2024

Do you mean Sidekiq::Testing.inline!? Faktory doesn't have a lot of language-specific infrastructure because it's meant to be a language-independent server. What we are discussing would be a FWG feature which skips Faktory entirely.

@adsteel
Copy link

adsteel commented Jan 11, 2024

Do you mean Sidekiq::Testing.inline!?

Yep!

Faktory doesn't have a lot of language-specific infrastructure because it's meant to be a language-independent server. What we are discussing would be a FWG feature which skips Faktory entirely.

I understand. I'll figure something out in that case. Thank you!

@mperham
Copy link
Collaborator

mperham commented Jan 11, 2024

If you decide to add an inline mode to FWG, I'd welcome a PR or further discussion on how to do it.

@adsteel
Copy link

adsteel commented Jan 11, 2024

I might do either of those, thank you. If I decide I'd like more discussion before a PR, would a new issue work best, or would you prefer to have that discussion here?

@mperham
Copy link
Collaborator

mperham commented Jan 11, 2024

New issue is fine. Open something when you have something to discuss.

@adsteel
Copy link

adsteel commented Jan 11, 2024

Not sure this idea rises to the level of a new issue, and since I'm probably done for the day, I wonder what you think of this proposal.

If we/you make the faktoryworker.jobContext function public, I'm able to write an inline solution that isn't too terrible.

In pseudo code, step 1:

// my worker defines its own push method, e.g.
func push(job *faktory.Job) error {
  if viper.GetBool("faktory_inline") {
    return inlinePush(job)
  }
  
  return normalPush(job)
}

Step 2, I have defined a registry of job type -> job function mappings:

registry := map[string]faktoryworker.Perform{...}

Step 3, if we have a public JobContext function, I think that's all we need for a minimal inline push that's faithful to the actual behavior?

func syntheticPush(job *faktory.Job) error {
	fn, _:= registry[NamespacedJobID(job.Type)]
	mgr := faktory_worker.NewManager()

	runCtx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
	defer cancel()

	_ := mgr.RunWithContext(runCtx)

	jobCtx := faktory_worker.JobContext(mgr.Pool, job)

	_ = fn(jobCtx, job.Args...)
}

And if we wanted to make it possible to inline with middleware we'd have to also make dispatch and mgr.middlewares public.

Any initial thoughts on an approach in this direction?

@adsteel
Copy link

adsteel commented Jan 12, 2024

After sleeping on it, I realized my suggestion above is a light touch approach to the current code (requires minimal library changes) but won't necessarily provide the simple implementation we might want. Another psuedo code solution that provides easier implementation and more library control might package this logic up on the manager and look like this:

if mgr.HasRegisteredJob(job) {
  err := mgr.ImmediatelyExecuteJob(job)
}

under the hood might look something like

func (mgr *Manager) ImmediatelyExecuteJob(job *faktory.Job) error {
  perform, ok := mgr.jobHandlers[job.Type]
  if !ok {
    // ...
  }
  return dispatch(mgr.middleware, jobContext(mgr.Pool, job), job, perform)
}

@mperham
Copy link
Collaborator

mperham commented Jan 12, 2024

That's a good start. If you want to try it out and send me a PR if you're happy with it, we can work on getting it into FWG.

@adsteel
Copy link

adsteel commented Jan 12, 2024

Okay, draft PR opened without tests. Let me know what you think. contribsys/faktory_worker_go#72

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

No branches or pull requests

3 participants