-
Notifications
You must be signed in to change notification settings - Fork 251
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
feat(test): introduce a base class for all TransferProcessStore #1982
feat(test): introduce a base class for all TransferProcessStore #1982
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1982 +/- ##
==========================================
+ Coverage 62.98% 63.01% +0.03%
==========================================
Files 784 786 +2
Lines 16671 16643 -28
Branches 1085 1086 +1
==========================================
- Hits 10500 10488 -12
+ Misses 5718 5702 -16
Partials 453 453
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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 noticed that the SQL mapping and the tests use a wrong property name: the use TransferProcess.dataAddress
, where in truth the property is called TransferProcess.contentDataAddress
. Checkout this gist to fix it. Once that's fixed, the map query should work automatically.
Also, since your at it, please rename TransferProcessStore.processIdForTransferId
, that method is named in a confusing way :)
import static org.junit.jupiter.api.Assertions.assertNotSame; | ||
import static org.junit.jupiter.api.Assertions.assertNull; | ||
|
||
public abstract class TransferProcessStoreTest { |
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.
Should probably be named ...TestBase
to keep consistency
var supportedOperators = getSupportedOperators(); | ||
boolean hasLikeOperator = true; | ||
boolean hasInOperator = true; | ||
if (!supportedOperators.isEmpty()) { |
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.
we probably should harmonize this: i.e. have a boolean supportsXyzOperator()
method per operator
3531cb5
to
426e629
Compare
What this PR changes/adds
Add a common base test class for TransferProcessStore which all implementors should extends
Why it does that
To provide a "least common denominator" for tests, i.e. a minimum subset of test scenarios for each TransferProcessStore.
Further notes
A fix has been added to
BaseSqlDialectStatements
fo transferprocess store.In the method
getProcessIdForTransferIdTemplate()
the filtered column has been changed fromgetProcessIdColumn
togetDataRequestIdColumn
which seems to be the same as InMemory and Cosmos implementationsLinked Issue(s)
Closes #1942
Checklist
no-changelog
)