-
Notifications
You must be signed in to change notification settings - Fork 1
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
DEVEXP-530: Conversation e2e tests (Apps) #135
DEVEXP-530: Conversation e2e tests (Apps) #135
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned Manifest Files.github/workflows/build.yml
pom.xml
.github/workflows/maven.yml
|
Assertions.assertTrue(deletePassed); | ||
} | ||
|
||
void checkExpectedAppResponseCommonFields(AppResponse appResponse) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice idea to factorize the check on common fields. But some tests can break if we change some data in the mockserver expectations. I've tried to create some consistency when I wrote them. Let's agree together to keep it like this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to our discussion: we agree to update tests accordingly in case of future change
public void delete() { | ||
|
||
service.delete("foo"); | ||
deletePassed = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you are just checking that the request completed without exception right?
Note that if we decide to move to static values instead of regexp for parameter matching, this test will fail as foo
is certainly not a value corresponding to an appId. You may reuse the one tou are already checking in the common fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to our discussion: we agree to update tests accordingly in case of future change
Based onto Cucumber and Sinch SDK MockServer: e2e for Conversation/Apps
(
build.yml
appear as a new file but it was renamed and enhanced)Notes: runing tests in multithreading mode based onto a single SinchClient instance
Tests output: