OBGM-328 Reference implementation for transaction type unit tests

[chetanmaharishi]

Unit test case for Transaction Type

[EWaterman]

what does the empty domain do here? What case is this testing?

[chetanmaharishi]

we are testing for empty domain. so it can throw error for all fields that has some validation. So in future if add any new field that can be catch here.

[jmiranda]

Basically, so we can check all of the errors at once in case we accidentally added a nullable:false validation that isn't being tested elsewhere. This should also catch validation that was removed. We should make sure this test includes a check on the actual fields / error codes returned in the Errors object, not just the count.

[EWaterman]

okay that makes sense. I like the idea. It forces you to consider tests when you make a schema change. Though I agree it's probably a good idea to at least assert on each of the errors instead of just the number of errors

[jmiranda]

Yes, that makes sense as you may have removed one validation rule and added another.

[EWaterman]

What does the console output look like after running the tests if you do something like:

    @Test
    @Unroll
    void 'TransactionType.validate() with name: #value should return #expected with errorCode: #expectedErrorCode'() {
    ...
}

If it's the same, then I'd prefer this format since it's redundant to need to specify the test name twice

[jmiranda]

I would also like to see a more readable BDD expression for the test methods. Something like this would be infinitely more readable to me even if it's missing details like the target method. This also makes the test case easier to search.

Source: https://medium.com/version-1/spock-for-bdd-unit-testing-in-java-d67c2ddf25ad

void "Should return #expected with errorCode: #expectedErrorCode when name is #value"() { 
    ...
}

[jmiranda]

@EWaterman I made some modifications to the tests and this is what the output looks like in the console.

and the test report

If we don't want data (especially long names) in the test spec description, then we could probably break the tests into "test with valid data" vs "tests against invalid data"

So, instead of this

org.pih.warehouse.inventory.TransactionTypeSpec > Should return false with error code nullable when validating transaction type given name null PASSED

org.pih.warehouse.inventory.TransactionTypeSpec > Should return false with error code maxSize.exceeded when validating transaction type given name aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa PASSED

org.pih.warehouse.inventory.TransactionTypeSpec > Should return true with error code null when validating transaction type given name bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb PASSED

we'd see this

org.pih.warehouse.inventory.TransactionTypeSpec > Should return false with error code nullable when validating transaction type given an invalid name PASSED

org.pih.warehouse.inventory.TransactionTypeSpec > Should return false with error code maxSize.exceeded when validating transaction type given an invalid name PASSED

org.pih.warehouse.inventory.TransactionTypeSpec > Should return true with error code null when validating transaction type given a valid name PASSED

Or we could add a test description to the table

adjective    || value        || expected | expectedErrorCode
null         || null         || false    | 'nullable'
really long  || 'a' * 256    || false    | 'maxSize.exceeded'
longish      || 'b' * 255    || true     | null
valid        || 'Dummy Name' || true     | null

So the test spec

def 'Should return #expected with error code #expectedErrorCode when validating transaction type given a #adjective name'() {

would resolve to

org.pih.warehouse.inventory.TransactionTypeSpec > Should return false with error code maxSize.exceeded when validating transaction type given a really long PASSED

[EWaterman]

yeah I agree in that I don't think we necessarily need to be ultra-verbose in specifying the exact inputs in the test name, but also I'm not sure if I'm a fan of needing to specify additional adjective fields just to populate the test name properly. I'm not sure it's the hugest deal either way. Ultimately I feel like as long as we stick to a general "X should return/do Y when Z" format, we're good.

[chetanmaharishi]

These are final results after running test cases

1. org.pih.warehouse.inventory.TransactionTypeSpec > should return false with error code nullable when validating transaction type given name null PASSED

2. org.pih.warehouse.inventory.TransactionTypeSpec > should return false with error code maxSize.exceeded when validating transaction type given name aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa PASSED

3. org.pih.warehouse.inventory.TransactionTypeSpec > should return true with error code null when validating transaction type given name bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
   bbbbbbbbbbbbbbbbbbbbb PASSED

4. org.pih.warehouse.inventory.TransactionTypeSpec > should return true with error code null when validating transaction type given name Dummy Name PASSED

5. org.pih.warehouse.inventory.TransactionTypeSpec > should return true with error code null when validating transaction type given description null PASSED

6. org.pih.warehouse.inventory.TransactionTypeSpec > should return false with error code maxSize.exceeded when validating transaction type given description aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa PASSED

7. org.pih.warehouse.inventory.TransactionTypeSpec > should return true with error code null when validating transaction type given description Dummy description PASSED

8. org.pih.warehouse.inventory.TransactionTypeSpec > should expect two validation errors when validating given an empty transaction type PASSED

9. org.pih.warehouse.inventory.TransactionTypeSpec > should expect the names to be the same PASSED

10. org.pih.warehouse.inventory.TransactionTypeSpec > should return false with error code nullable when validating transaction type given transactionCode null PASSED

11. org.pih.warehouse.inventory.TransactionTypeSpec > should return true with error code null when validating transaction type given transactionCode DEBIT PASSED

12. org.pih.warehouse.inventory.TransactionTypeSpec > should return true when given name Adjustment|fr:Ajustement|es:Ajustamiento PASSED

13. org.pih.warehouse.inventory.TransactionTypeSpec > should return false when given name NO_Adjustment PASSED

14. org.pih.warehouse.inventory.TransactionTypeSpec > should return true when given name Adjustment PASSED

15. org.pih.warehouse.inventory.TransactionTypeSpec > should return Wed May 29 22:43:35 IST 2024 and Wed May 29 22:43:35 IST 2024 when given name Testing Name and transactionCode DEBIT and
    dateCreated Wed May 29 22:43:35 IST 2024 and lastUpdated Wed May 29 22:43:35 IST 2024 PASSED

[EWaterman]

You shouldn't need to add The Junit @Test annotation because these are Spock tests.

Also, you don't need @Unroll unless there's a where: block