Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat: add integration testing for Rust connection node #1105

Merged
merged 1 commit into from
Jan 18, 2018

Conversation

bbangert
Copy link
Member

Adds the following functionality for capability parity:

  • Skip sending messages if the message is expired
  • Properly handle legacy messages in a message stream
  • Set appropriate flags for a uaid not found in the db
  • Always return a timestamp when querying into timestamp messages
  • Send messages in the order they're retrieved from the db
  • Accept messages from endpoint while waiting for acks
  • Don't save TTL:0 messages in the db if the client fails to ack them
  • Allow TTL:None from endpoint and treat as TTL:0

Closes #1060

@bbangert bbangert force-pushed the feat/issue-1060 branch 2 times, most recently from 5df458a to c5a5d20 Compare January 18, 2018 18:25
@codecov-io
Copy link

codecov-io commented Jan 18, 2018

Codecov Report

Merging #1105 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
+ Coverage   99.67%   99.67%   +<.01%     
==========================================
  Files          58       58              
  Lines        9321     9325       +4     
==========================================
+ Hits         9291     9295       +4     
  Misses         30       30
Impacted Files Coverage Δ
autopush/tests/test_z_main.py 100% <ø> (ø)
autopush/tests/test_integration.py 100% <100%> (ø) ⬆️

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 5b4cd09...c4351cc. Read the comment docs.

alexcrichton
alexcrichton previously approved these changes Jan 18, 2018
@@ -1,3 +1,9 @@
"""Test main instantiation
Copy link
Member

Choose a reason for hiding this comment

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

Will you file an issue to clean up these tests?

Copy link
Member

Choose a reason for hiding this comment

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

I got it: #1106

Some(Box::new(ClientState::SendMessages(Some(messages)))),
)
// Filter out TTL expired messages
let now = time::get_time().sec as u32;
Copy link
Member

@jrconlin jrconlin Jan 18, 2018

Choose a reason for hiding this comment

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

In rust, we use timestamps of u32, but python has timestamps of u64. Do we need to cast up to prevent possible overruns?

Copy link
Member Author

Choose a reason for hiding this comment

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

This particular timestamp is seconds precision only as its just used for the TTL comparison.

jrconlin
jrconlin previously approved these changes Jan 18, 2018
Copy link
Member

@jrconlin jrconlin left a comment

Choose a reason for hiding this comment

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

pending travis resub

@@ -271,7 +276,13 @@ where
uaid,
use_webpush: Some(true),
..
} => uaid,
} => {
if let Some(uaid) = uaid {
Copy link
Member

Choose a reason for hiding this comment

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

this could be uaid.and_then(|uaid| Uuid::parse_str(uaid.as_str()).ok())

use_webpush: Some(true) isn't necessary in the destructuring above either is it

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I knew there was something shorter I was missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the Some(true) is needed to enforce that it was set to True?

Copy link
Member

Choose a reason for hiding this comment

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

I guess but it's never false anyway

@bbangert bbangert dismissed stale reviews from jrconlin and alexcrichton via 26e8e88 January 18, 2018 22:00
Adds the following functionality for capability parity:

- Skip sending messages if the message is expired
- Properly handle legacy messages in a message stream
- Set appropriate flags for a uaid not found in the db
- Always return a timestamp when querying into timestamp messages
- Send messages in the order they're retrieved from the db
- Accept messages from endpoint while waiting for acks
- Don't save TTL:0 messages in the db if the client fails to ack them
- Allow TTL:None from endpoint and treat as TTL:0

Closes #1060
@bbangert bbangert merged commit a519f23 into master Jan 18, 2018
@bbangert bbangert deleted the feat/issue-1060 branch January 18, 2018 23:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants