refactor(core): Update dynamic node parameter endpoints to use DTOs #12379
Conversation
netroy
force-pushed
the
CAT-455-port-over-more-dtos-part-4
branch
from
46a8d59 to
e7dc5da
Compare
n8n-assistant
bot
added
core
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
| const req = { user: mockUser } as AuthenticatedRequest; | ||
|
|
||
| const expectedResult: INodePropertyOptions[] = [{ name: 'test', value: 'value' }]; | ||
| service.getOptionsViaMethodName.mockResolvedValue(expectedResult); |
There was a problem hiding this comment.
question: Does not mocking the service here defeats the whole purpose of a e2e test? If we break getOptionsViaMethodName this test will pass even when it should no?
There was a problem hiding this comment.
I guess if we had tests for DynamicNodeParametersService, it will be ok, but currently we do not 🤔
There was a problem hiding this comment.
that's definitely a problem. quite a lot of our integration tests are using mocks. I tried fixing it here, but it very quickly became a side quest of its own. I'll address this in a separate PR.
|
|
n8n
|
||||||||||||||||||||||||||||||||||
| Project |
n8n
|
| Branch Review |
CAT-455-port-over-more-dtos-part-4
|
| Run status |
|
| Run duration | 04m 08s |
| Commit |
|
| Committer | कारतोफ्फेलस्क्रिप्ट™ |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
1
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
277
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
48-subworkflow-inputs.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Sub-workflow creation and typed usage > works with Fields input source into JSON input source |
Test Replay
Screenshots
Video
|
|
The first 5 failed specs are shown, see all 51 specs in Cypress Cloud.
|
|
|
|
Summary
This is part 4 in a series of refactors.
Previous PRs:
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)