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.

Sign up for GitHub

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

Jump to bottom

IWF-438: Validate if RPC method is not final #287

Merged
merged 7 commits into from
Dec 31, 2024
Merged

IWF-438: Validate if RPC method is not final #287

merged 7 commits into from
Dec 31, 2024

Conversation

lwolczynski
Copy link
Contributor

@lwolczynski lwolczynski commented Dec 30, 2024 •
edited
Loading

Description

Checklist

  • Code compiles correctly
  • Tests for the changes have been added
  • All tests passing
  • This PR change is backwards-compatible
  • This PR CONTAINS a (planned) breaking change (it is NOT backwards-compatible)

Related Issue

Closes #286

@lwolczynski lwolczynski force-pushed the jira/lwolczynski/IWF-438 branch from 961c032 to d33573e Compare December 30, 2024 15:30
samuel27m
samuel27m approved these changes Dec 30, 2024
View reviewed changes
Copy link
Member

@samuel27m samuel27m left a comment

Choose a reason for hiding this comment

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

LGTM! Is there a way to add Unit Testing for this? I'm guessing the answer is no, but always good to double-check.

@lwolczynski
Copy link
Contributor Author

LGTM! Is there a way to add Unit Testing for this? I'm guessing the answer is no, but always good to double-check.

I think there can be. Let me try!

@lwolczynski lwolczynski force-pushed the jira/lwolczynski/IWF-438 branch from c74cf40 to 6dac485 Compare December 30, 2024 18:40
@indeedeng indeedeng deleted a comment from codecov bot Dec 30, 2024
@lwolczynski
Copy link
Contributor Author

LGTM! Is there a way to add Unit Testing for this? I'm guessing the answer is no, but always good to double-check.

I think there can be. Let me try!

That would be more complicated than expected. Adding a workflow with a finalized RPC method causes the service to crash at the start, which makes everything fail. See this commit: c210761

To implement the test, I would need to spin up two Spring Boot apps: one that implements a finalized RPC method and another without it. Then, the first one would be only created to verify that it fails at the start, and the other one would be used for all existing tests. This seems to be somewhat complicated and not worth investing the required effort

@samuel27m
Copy link
Member

LGTM! Is there a way to add Unit Testing for this? I'm guessing the answer is no, but always good to double-check.

I think there can be. Let me try!

That would be more complicated than expected. Adding a workflow with a finalized RPC method causes the service to crash at the start, which makes everything fail. See this commit: c210761

To implement the test, I would need to spin up two Spring Boot apps: one that implements a finalized RPC method and another without it. Then, the first one would be only created to verify that it fails at the start, and the other one would be used for all existing tests. This seems to be somewhat complicated and not worth investing the required effort

Makes sense! Thanks so much for looking into that

@lwolczynski lwolczynski merged commit 8003927 into jira/holiday/IWF-435 Dec 31, 2024
1 check passed
lwolczynski added a commit that referenced this pull request Jan 2, 2025
@lwolczynski
IWF-438: Validate if RPC method is not final (#287)
27b28d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuel27m samuel27m samuel27m approved these changes

@stevo89519 stevo89519 Awaiting requested review from stevo89519

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lwolczynski @samuel27m