Skip to content

Add deeper tests for DDApi using ratpack as a mock http server. #31

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 1 commit into from
Jul 5, 2017

Conversation

tylerbenson
Copy link
Contributor

@tylerbenson tylerbenson commented Jul 3, 2017

See https://danhyun.github.io/2016-gr8confus-testing-ratpack-apps/ for more examples with this style of testing.
For more details about spock, see here: http://spockframework.org/spock/docs/1.0/spock_primer.html

@tylerbenson tylerbenson requested a review from gpolaert July 3, 2017 22:00
Copy link
Contributor

@gpolaert gpolaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are fine.

Just question about groovy tests, Should we use groovy?

My concerns are about mixing syntaxes, learning curve and maintenance in the long term.

I think we have to be sure using groove is a huge plus for the project. Otherwise, I think we should have keep only one syntax.

It's just my opinion. Tyler, you will(are ;) ) the lead, so it's your call.

import com.datadoghq.trace.DDSpan
import com.datadoghq.trace.DDSpanContext

class SpanFactory {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitively a great idea :)

@tylerbenson
Copy link
Contributor Author

tylerbenson commented Jul 5, 2017

I totally understand where you're coming from because a year ago I expressed similar opinions to a coworker who was trying to introduce Spock on a project. Eventually I tried it on a project and became a big convert. I find Spock tests to be much more readable and concise. It is also nice using Groovy as it removes a lot of the verbosity usually required by Java. Normally I wouldn't try and push something like this mid way through a project, but we can migrate slowly and the project is still very young.

@tylerbenson tylerbenson merged commit 70e43a1 into master Jul 5, 2017
@tylerbenson tylerbenson deleted the tyler/testing branch July 5, 2017 15:59
This was referenced May 20, 2025
This was referenced Jun 6, 2025
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