Fix test suite to properly handle async specs #109
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This test was previously failing, but async, outside of the test cycle. It was also broken, because it did not toggle open the general tab; so hovering over the actual `versionTag` field could never happen. Now we clearly model the user journey. Bonus points for getting rid of `clock`. We can simply use `waitFor` to wait for a DOM element expected for arrival.
nikku
requested review from
a team,
jarekdanielak and
barmac
and removed request for
a team
bpmn-io-tasks
bot
added
in progress
|
Converted to draft as this needs further "action". |
nikku
added
ready
|
All tests are now green. ✅ We (me) changed some fields in the properties panel from inputs to text areas and forgot to adjust in the tests. I also added some values to the fields in fixture not to compare against undefined -> 5873297 Rest of the changes are about some incorrect element ID or wrong bootstrapping of the modeler. |
bpmn-io-tasks
bot
added
needs review
barmac
reviewed
Dec 18, 2024
| @@ -116,10 +116,10 @@ describe('provider/dmn - IdProps', function() { | |||
| it('should set invalid', inject(async function(elementRegistry, selection) { | |||
|
|
|||
| // given | |||
| const task = elementRegistry.get('Task_1'); | |||
|
I've pushed 93a903a to not print the XML on CI/when all tests are run, but overall the PR looks good. Great job! |
barmac
approved these changes
Dec 18, 2024
|
Thanks @jarekdanielak for taking this on 💪 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a matter of fact the test suite to date does not correctly handle async specs like this, a prominent (and used) pattern:
I fixed this via c9ee643.
Of course this covers up a bunch (24 / 130) broken tests. One test (verifying version tag tooltip) was completely broken, used a non-existing utility, and was structured so it would never work. The test is now properly architected, and works, modeling a user flow:
I did not touch the other 24 failing tests, but I suggest we handle them ("make them work") in similar ways.