Feature/ultimate owner system tests - #1782
Conversation
…HQ ownership checks effective
…via IsAlternativeHeadquarterFor relations (eclipse-tractusx#1743)
…dBy ownership on alternative
…StreetScriptVariant.empty
# Conflicts: # bpdm-pool/src/main/kotlin/org/eclipse/tractusx/bpdm/pool/mapper/poolv6/outbound/LegalEntityParseErrorMapperV6.kt # bpdm-pool/src/main/kotlin/org/eclipse/tractusx/bpdm/pool/service/AlternativeHeadquarterRelationUpsertService.kt # bpdm-pool/src/main/kotlin/org/eclipse/tractusx/bpdm/pool/service/parser/AlternativeHeadquarterValidator.kt # bpdm-pool/src/main/kotlin/org/eclipse/tractusx/bpdm/pool/service/parser/LegalEntityUpdateParser.kt # docs/admin/MIGRATION_GUIDE.md
nicoprow
left a comment
nicoprow
left a comment
There was a problem hiding this comment.
The step definitions for the cucumber files do not exist in the system tester code and therefore the tests fail. The following step definitions are missing:
- the {parent|grandparent|self-owner} entity {string} is marked as ownershipUltimate = true in the golden record
- the golden record process processes the update
- {string} output reflects ultimateOwnerBpnl as {string} / ... as null
Step 1 and 3 should be legitimitately implemented. Step 2 is unnecessary because the test helper methods already wait for the update to process. So all 'proccesses update' steps should be removed.
For executing the tests, the most straight forward way is to rebuild the system tester jar and then execute:
java -jar bpdm-system-tester/target/bpdm-system-tester.jar \
--threads 5 \
classpath:cucumber/ultimate_owner_distribution.featureYou can also use intellij, find the system tester's Application.kt file, right click and then choose to run the Application.kt's main method. But that runs all tests and could result in a longer waiting time.
…itions - Mark entity as ultimate owner with ownershipUltimate flag - Assert ultimateOwnerBpnl values in output - Remove redundant golden record process steps
nicoprow
left a comment
nicoprow
left a comment
There was a problem hiding this comment.
The branch could use a merge from main. Other than that the test scope looks fine. The tests need more work in being more user flow representative. Also, step definitions could be more in 'business' talk.
| legalEntity = updatedLegalEntity | ||
| ) | ||
|
|
||
| poolClient.legalEntities.updateBusinessPartners(listOf(updateRequest)) |
There was a problem hiding this comment.
This is not representative of the user flow. In a E2E scenario the sharing member designates an ultimate owner over the gate. This should be done here as well instead of taking an unrepresentative shortcut.
| And record "child-record" reflects legal entity "child" | ||
| When the sharing member shares relation "ownership" of type "IsOwnedBy" from "child-record" to "parent-record" | ||
| And the golden record process establishes relation "ownership" | ||
| And the parent entity "parent" is marked as ownershipUltimate = true in the golden record |
There was a problem hiding this comment.
This here is again two steps: The sharing member marks the legal entity as ultimate owner and the golden record process needs to confirm(establish) that the legal entity is indeeo the ultimate owner. Two steps, one for sharing member int gate, one for the golden record process service provider in the orchestrator.
For the naming there is a Gherkin convention that you try to talk 'business' here and not leak implementation details. Try not to use field names in the step definitions. Instead of 'ownershipUltimate = true' you can just use a description like 'marked as ultimate owner'.
| And the sharing member shares relation "child-ownership" of type "IsOwnedBy" from "child-record" to "parent-record" | ||
| And the golden record process establishes relation "child-ownership" | ||
| And the grandparent entity "grandparent" is marked as ownershipUltimate = true in the golden record | ||
| Then "parent-record" output reflects ultimateOwnerBpnl as "grandparent" |
There was a problem hiding this comment.
Also here, I would not use the field name but just basically write it out and it becomes 'business' talk: 'reflects "grandparent" as the ultimate owner'.
| And record "child-record" reflects legal entity "child" | ||
| When the sharing member shares relation "ownership" of type "IsOwnedBy" from "child-record" to "parent-record" | ||
| And the golden record process establishes relation "ownership" | ||
| Then "child-record" output reflects ultimateOwnerBpnl as null |
There was a problem hiding this comment.
reformulate to something more business here as well. 'has no ultimate owner' for example
…per user flow Use Gate API for sharing member to mark ultimate owner, add two-step process, replace technical field names with business language
Use LegalEntityRepresentationInputDto instead of LegalEntityDto
Use header.legalName, header.legalShortName, header.legalFormVerbose?.technicalKey, and header.states
2 participants
Description
ultimate owner system tests
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: