Skip to content

dpservice-cli: fix for pytest unable to detect errors#657

Merged
guvenc merged 1 commit intomainfrom
dpservice-cli/fix-pytest
Mar 25, 2025
Merged

dpservice-cli: fix for pytest unable to detect errors#657
guvenc merged 1 commit intomainfrom
dpservice-cli/fix-pytest

Conversation

@vlorinc
Copy link
Copy Markdown
Contributor

@vlorinc vlorinc commented Mar 19, 2025

Proposed Changes

  • Changes in error rendering: errors that are returned to main are parsed and rendered based on output flag, if json output was requested, error will now be rendered as json (using JsonError struct similar to other objects)
  • Pytest was updated to reflect new output formats
  • Pytest now also always checks for response from grpc client
  • Error object also includes "source" field that indicates where the error originated (client, grpc, server)

Fixes #650

@vlorinc vlorinc requested a review from a team as a code owner March 19, 2025 14:53
@vlorinc vlorinc linked an issue Mar 19, 2025 that may be closed by this pull request
@github-actions github-actions bot added bug Something isn't working size/L labels Mar 19, 2025
Copy link
Copy Markdown
Contributor

@PlagueCZ PlagueCZ left a comment

Choose a reason for hiding this comment

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

This seems to work really well, the new approach is better. But I was part of the change, so we need a fresh a pair of eyes now.

@PlagueCZ PlagueCZ requested a review from guvenc March 19, 2025 23:04
all errors returned to main are parsed and rendered based on output flag
e.g. if json output was requested error will also be shown as json
Update pytest to latest dpservice-cli changes, add tests for error responses
improved error parsing and error source is now generated automatically
fix return value in ListNats function
@vlorinc vlorinc force-pushed the dpservice-cli/fix-pytest branch from eb2e512 to 4d94cff Compare March 20, 2025 12:38
Copy link
Copy Markdown
Contributor

@guvenc guvenc left a comment

Choose a reason for hiding this comment

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

LGTM. I also liked the fact that we added some tests which would catch if the errors get ignored in a similar way.

@guvenc guvenc merged commit 92981a3 into main Mar 25, 2025
6 checks passed
@guvenc guvenc deleted the dpservice-cli/fix-pytest branch March 25, 2025 13:57
@hardikdr hardikdr added this to Roadmap Jun 26, 2025
@hardikdr hardikdr moved this to Done in Roadmap Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/networking bug Something isn't working size/L

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Pytest unable to detect errors

4 participants