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
fix(candeletecardinalities): return correct response on route negative case (DEV-36) #1910
Conversation
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.
I like the removal of duplicated code. Did you test if it is working? Because there is now a new clientTestDataCollector
created in each call of CollectClientTestData(...)
. Before, it was one clientTestDataCollector
for all the tests in this file. Do we still get all test data in the end?
@@ -78,7 +78,20 @@ class OntologyV2R2RSpec extends R2RSpec { | |||
private val clientTestDataPath: Seq[String] = Seq("v2", "ontologies") | |||
|
|||
// Collects client test data | |||
private val clientTestDataCollector = new ClientTestDataCollector(settings) | |||
// TODO: redefine below method somewhere else if can be reused over other test files | |||
private def CollectClientTestData(filename: String, payload: String) = { |
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.
private def CollectClientTestData(filename: String, payload: String) = { | |
private def collectClientTestData(filename: String, filecontent: String) = { |
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.
@irinaschubert at the end it's both payload and the file content ;) but in the matter of consistency I accept this suggestion.
val clientTestDataCollector = new ClientTestDataCollector(settings)
can be also defined outside the method, maybe this is better performance wise. Everything was working well, but I think that some tests failed after that commit 2d9c26d where we added 3 missing test data.
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.
LGTM, thanks!
private val clientTestDataCollector = new ClientTestDataCollector(settings) | ||
|
||
// Collects client test data | ||
// TODO: redefine below method somewhere else if can be reused over other test files | ||
private def CollectClientTestData(fileName: String, fileContent: String): Unit = |
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.
good idea. thanks!
resolves DEV-36