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

IT test for long values #713

Merged
merged 24 commits into from Mar 29, 2022
Merged

IT test for long values #713

merged 24 commits into from Mar 29, 2022

Conversation

tanvigour
Copy link
Contributor

Description

Please explain the changes you've made

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #686

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: tanvigour <tanvi.gour@gmail.com>
@tanvigour tanvigour requested review from a team as code owners March 23, 2022 15:17
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
"messages/testinglongvalues",
null,
HttpExtension.GET, CLOUD_EVENT_LIST_TYPE_REF).block();
assertEquals(590518626939830271L, messages.get(0).getData());
Copy link
Contributor

Choose a reason for hiding this comment

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

@tanvigour Why do we have to publish N messages and then check only one message ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can check for all values, just added a commit for that

Copy link
Contributor

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

overall lgtm ... if possible test for larger values of long and varied values ....

tanvigour and others added 4 commits March 24, 2022 11:44
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
@mukundansundar
Copy link
Contributor

@tanvigour
update workflow files to point to the right dapr commit which has the fix ...

Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Copy link
Contributor

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

@tanvigour Please address these comments.

@@ -413,4 +467,13 @@ public void setId(String id) {
this.id = id;
}
}

public static class ConvertToLong {
public Long value;
Copy link
Contributor

Choose a reason for hiding this comment

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

private

public void setVal(Long value) {
this.value = value;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add getter for value, and equals and Dashcode method (Can autogenerate in IntelliJ using Objects.hash etc ).

}

ConvertToLong value = new ConvertToLong();
value.setVal(590518626939830271L);
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a Random object to generate random values with 590518626939830271L as seed.
Create a Set HashSet,
Add 590518626939830271L as the first element.
Add more random long values upto NUM_MESSAGES size.

client.publishEvent(
PUBSUB_NAME,
LONG_TOPIC_NAME,
value,
Copy link
Contributor

Choose a reason for hiding this comment

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

create iterator of the set, and send the next element here.

daprRun.getAppName(),
"messages/testinglongvalues",
null,
HttpExtension.GET, CLOUD_EVENT_LIST_TYPE_REF).block();
Copy link
Contributor

Choose a reason for hiding this comment

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

ADD an new TypeRef
private static final TypeRef<List<CloudEvent<ConvertToLong>>> CLOUD_EVENT_LONG_LIST_TYPE_REF = new TypeRef<>() {};
use that here .

null,
HttpExtension.GET, CLOUD_EVENT_LIST_TYPE_REF).block();
for (int i = 0; i < NUM_MESSAGES; i++) {
System.out.println("The long value received!!! " +messages.get(i).getData().value);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove log.

"messages/testinglongvalues",
null,
HttpExtension.GET, CLOUD_EVENT_LIST_TYPE_REF).block();
for (int i = 0; i < NUM_MESSAGES; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (int i = 0; i < NUM_MESSAGES; i++) {
Assert.assertNotNull(messages);
for (int i = 0; i < messages.size(); i++) {

HttpExtension.GET, CLOUD_EVENT_LIST_TYPE_REF).block();
for (int i = 0; i < NUM_MESSAGES; i++) {
System.out.println("The long value received!!! " +messages.get(i).getData().value);
assertEquals(590518626939830271L, messages.get(i).getData().value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add it to the actual ConvertToLong Set created above.

actual.add(message.getData()

assertEquals(590518626939830271L, messages.get(i).getData().value);
}
}, 2000);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add assert comparing two set values

Assert.assertEquals(expected, actual);

Both expected and actual are of type Set<ConvertToLong

@@ -402,6 +404,58 @@ public void testPubSubTTLMetadata() throws Exception {
daprRun.stop();
}

@Test
public void testlongValues() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public void testlongValues() throws Exception {
public void testLongValues() throws Exception {

Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
@mukundansundar
Copy link
Contributor

mukundansundar commented Mar 28, 2022

Tested this locally ... it is working ... with latest master Dapr after contrib 1.7-rc merge

Confirmed listening on port 58606.
Dapr application started.
The long value sent 590518626939830271
== APP == Subscriber got: 590518626939830271
The long value sent 9162974433530815171
== APP == Subscriber got: 9162974433530815171
The long value sent 2803002362178450485
== APP == Subscriber got: 2803002362178450485
The long value sent 1005463900081880990
== APP == Subscriber got: 1005463900081880990
The long value sent 4093312918519397196
== APP == Subscriber got: 4093312918519397196
The long value sent 5939038007520118028
== APP == Subscriber got: 5939038007520118028
The long value sent 7145089449735285569
== APP == Subscriber got: 7145089449735285569
The long value sent 4024458962230509193
== APP == Subscriber got: 4024458962230509193
The long value sent 2837854465277681257
== APP == Subscriber got: 2837854465277681257
The long value sent 562839771211708662
== APP == Subscriber got: 562839771211708662
Checking results for topic testinglongvalues

@tanvigour is the test working for you locally ?

…ue to set

Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
@codecov
Copy link

codecov bot commented Mar 29, 2022

Codecov Report

Merging #713 (ddcd2b3) into master (c7cc94d) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #713   +/-   ##
=========================================
  Coverage     78.19%   78.19%           
  Complexity     1117     1117           
=========================================
  Files            97       97           
  Lines          3417     3417           
  Branches        399      399           
=========================================
  Hits           2672     2672           
  Misses          547      547           
  Partials        198      198           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7cc94d...ddcd2b3. Read the comment docs.

Copy link
Contributor

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

lgtm
cc @artursouza

@artursouza artursouza merged commit 1ce6738 into dapr:master Mar 29, 2022
artursouza pushed a commit to pkedy/java-sdk that referenced this pull request Jun 22, 2022
* run IT test for long values

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* Fix the class name

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* assert for all messages and fix class name

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* test for Long.MAX_VALUE

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* revert back long number and print

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* fix the typo

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* update the latest dapr commit in workflow files

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* log some more data to debug

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* debug

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* get the value from messages

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* address feedback

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* change the assertion

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* Add hashcode and equals functione and create new obj while adding value to set

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* move iterator

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* log value before adding to hashset

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* log value before adding to hashset

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* log value before adding to hashset

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* change assertion

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* fix equals method

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* adding debugging for expected value

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* adding logs for expected value

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

* log value before adding to hashset

Signed-off-by: tanvigour <tanvi.gour@gmail.com>

Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Artur Souza <artursouza.ms@outlook.com>
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.

Long values are not sent correctly with PubSub
3 participants