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

run_all_test_files event #52

Closed
wants to merge 1 commit into from
Closed

Conversation

joonty
Copy link
Contributor

@joonty joonty commented May 13, 2013

A potential implementation for #50.

A flag is passed from the driver to the engine to identify whether a
series of test files were queued as a result of the user triggering a
run of all the test files.

To accomplish this, I added a flag to Engine#run_test_files that states whether it was triggered by the run_all_test_files event. This flag is passed in Driver#run_all_test_files.

It feels somewhat awkward and highly specific, but I can't think of any other way of being able to trigger this event, as the running of test files is abstracted in the Driver class (which makes perfect sense).

So, it's a working solution, but one that you may or may not think could be improved upon.

a flag is passed from the driver to the engine to identify whether a
series of test files were queued as a result of the user triggering a
run of all the test files.
@sunaku
Copy link
Owner

sunaku commented May 14, 2013

@joonty did you get a chance to try commit 8ca66ca in the "echo" branch? I would prefer a generic solution like that.

@joonty
Copy link
Contributor Author

joonty commented May 15, 2013

Hmm. I think I misunderstood something you said in #50:

Yes, I tried echoing but since there are so many components running in parallel, there was a multiplicative effect: 1 message caused N echo responses.

I took that to mean that you got it working, but weren't happy with it because of the event overload. I therefore went ahead and specifically implemented these events.

No problem - if you don't want to take this approach then it's fine to throw these PRs away, I'm not precious about them :)

But what's the next step in your mind?

@sunaku
Copy link
Owner

sunaku commented May 16, 2013

Yes, that concern about multiplying messages was about a my first shot at
implementing the echo feature.

The "echo" branch now contains a better implementation which does not
suffer from the multiplying problem.

@joonty
Copy link
Contributor Author

joonty commented May 16, 2013

Ok, I'll close these PRs in that case. Thanks

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