Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve error message on mockStep failure #74

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

TWiStErRob
Copy link
Contributor

This helps to find which mock rule is not working. If there are multiple mockSteps, it's hard to figure out otherwise which one is wrong. This helps clarify it.

Before:

Error: Could not find step
    at StepMocker.mock (.../node_modules/@kie/act-js/build/src/step-mocker/step-mocker.js:35:27)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)

After:

Error: Could not find step {"uses":"actions/upload-artifact@v4"} in job unit-test
in /foo/bar/baz/tests/repo/testRepo/.github/workflows/ci.yml

    at StepMocker.mock (/foo/bar/baz/tests/node_modules/@kie/act-js/build/src/step-mocker/step-mocker.js:37:27)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)

@shubhbapna shubhbapna self-requested a review February 12, 2024 21:18
@TWiStErRob
Copy link
Contributor Author

@shubhbapna I think my last commit solves it, can you please trigger again?

Copy link
Collaborator

@shubhbapna shubhbapna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry just a nitpick but is it possible to fix the linter warning message?

@TWiStErRob
Copy link
Contributor Author

I don't know how, it must be named mockWith otherwise the destructuring doesn't work. Can we supress for that one line?


Also: consider making the lint fail on warnings ;)

@shubhbapna
Copy link
Collaborator

You could rename the destructuring:

const { mockWith: _mockWith, ...stepQuery } = mockStep;

Or adding this line of comment directly above it should also disable it:

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { mockWith, ...stepQuery } = mockStep;

Copy link
Collaborator

@shubhbapna shubhbapna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much!

@shubhbapna shubhbapna changed the title Improve error message on mockStep failure fix: Improve error message on mockStep failure Feb 13, 2024
@shubhbapna shubhbapna merged commit a096aab into kiegroup:main Feb 13, 2024
5 checks passed
@TWiStErRob TWiStErRob deleted the patch-1 branch February 13, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants