fix: remove import of non-existing API #581
Conversation
|
Looks good. What about backporting the utility to And another one: What about adding a test case that verifies we still run O.K. against bpmn-js@8. We've run into enough issues to justify that. |
That would be cool but we'd still depend on the most recent minor from 8.x series in that case.
I'll think about it tomorrow. |
47bdc01 to
9de938e
Compare
|
I've added integration test via 9de938e |
|
@marstamm and I just had a call to discuss the breaking tests on bpmn-js@8. I will adjust the tests in this PR and add a skip for the bpmn-js@9 special case of subprocess as the root element. |
e6df897 to
725908c
Compare
725908c to
589eda4
Compare
|
@nikku @marstamm This is ready for review. Apart from fixing the problem, it includes now a CI change which makes us run the whole test suite with bpmn-js@8 and diagram-js@7. We skip one test which can work only in the upcoming release of bpmn-js. As soon as bpmn-js@9 is out, we should adjust the version at to use the latest version. |
6a080ba to
e43a4cc
Compare
|
I've added 3a650e7 to make sure we don't upload coverage twice. |
| @@ -367,24 +368,28 @@ describe('provider/camunda-platform - ProcessVariableProps', function() { | |||
| })); | |||
|
|
|||
|
|
|||
| it('should display items for plane', inject(async function(elementRegistry, selection, canvas) { | |||
| testForBpmnJsVersion('>=9')('should display items for plane', | |||
There was a problem hiding this comment.
I'm personally a fan of CHECK && it('...', ...) to not hide what we're doing.
So to adopt to your example here:
bpmnJsVersion(`>=9`) && it('should display items for plane', ...
There was a problem hiding this comment.
Or, if this is a core use-case, simplify it:
import {
bpmnJs9
} from '...'
bpmnJs9() && it('test what works on nine only')
There was a problem hiding this comment.
All just personal preference. Happy to adopt what works for you.
There was a problem hiding this comment.
Hmm but in that case we hide information that the test case was skipped. WDYT?
There was a problem hiding this comment.
You got a point on the skip.
Then, to be less verbose why not include a simple it9 helper?
import {
it9
} from 'test-helper'
it9('do what works with bpmn-js nine+ only');
There was a problem hiding this comment.
I shortened this to withBpmnJs. it9 sounds super cryptic to me.
Co-authored-by: Martin Stamm <martin.stamm@camunda.com>
|
3a650e7 doesn't work in action because |
Closes #580