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: print path to invalid actor.json when an invalid file is found #618

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

vladfrangu
Copy link
Member

@vladfrangu vladfrangu commented Aug 26, 2024

Invalid actor.jsons will have their path printed out now

iTerm2 - 2024-08-27 at 18 03 20

Closes #561

@vladfrangu vladfrangu requested a review from B4nan August 26, 2024 13:53
@vladfrangu vladfrangu self-assigned this Aug 26, 2024
@github-actions github-actions bot added this to the 97th sprint - Tooling team milestone Aug 26, 2024
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Aug 26, 2024
Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

some test would be nice, i wonder how much time it takes to write the specs for this in cucumber so it would clearly show the output with the error? if its more than an hour, i wouldn't bother with that for this small thing though

or at least add a screenshot to the PR description showing the error in action

@vladfrangu
Copy link
Member Author

@B4nan I've added an image to the PR description. I'll also look into the cucumber test case, it should be easy enough

```
- Then I can read text on stderr:
```
Failed to read local config at path:
Copy link
Member

Choose a reason for hiding this comment

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

i wonder if this can contain some wildcards so we could also assert a part of the path too

Copy link
Member Author

Choose a reason for hiding this comment

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

It can contain anything we want it to contain, we'd just need to parse it in the handler for this matcher. I'd probably leave that for another PR unless you'd prefer it here

Copy link
Member

Choose a reason for hiding this comment

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

all right, we can keep this for later, but i can imagine this will be important in other places

@vladfrangu vladfrangu merged commit 8f92580 into master Aug 27, 2024
21 checks passed
@vladfrangu vladfrangu deleted the fix/nicer-error-messages-for-invalid-actor.json branch August 27, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cryptic error message when actor.json is an invalid JSON
2 participants