Skip to content

/ sync

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

Add SQS test #244

Merged
merged 3 commits into from Nov 17, 2018
Merged

Add SQS test #244

merged 3 commits into from Nov 17, 2018

Conversation

@darkdh
Copy link
Member

darkdh commented Nov 13, 2018

fix #220
@darkdh darkdh self-assigned this Nov 13, 2018
@darkdh darkdh requested review from diracdeltas and SergeyZhukovsky Nov 13, 2018
@darkdh darkdh force-pushed the sqs_test branch from 4b6015d to 0204f18 Nov 13, 2018
@diracdeltas
Copy link
Member

diracdeltas commented Nov 13, 2018

travis failing :O
@diracdeltas
diracdeltas reviewed Nov 13, 2018
View changes
test/client/requestUtil.js Outdated Show resolved Hide resolved
@darkdh
Copy link
Member Author

darkdh commented Nov 13, 2018

seems like failing with npm audit. tests and lints are both fine
@diracdeltas
diracdeltas reviewed Nov 13, 2018
View changes
test/client/requestUtil.js
@@ -175,12 +178,102 @@ test('client RequestUtil', (t) => {
t.deepEquals(s3Record.deviceId, record.deviceId, `${t.name}: deviceId`)
t.deepEquals(s3Record.objectId, record.objectId, `${t.name}: objectId`)
t.deepEquals(s3Record.historySite, record.historySite, `${t.name}: historySite.location`)
testCanDeleteHistorySites(t)

This comment has been minimized.

Sign in to view
Copy link
@diracdeltas

diracdeltas Nov 13, 2018

Member

why was this moved into a callback? seems that the two tests should be independent

This comment has been minimized.

Sign in to view
Copy link
@darkdh

darkdh Nov 13, 2018

Author Member

it is called after testCanDeleteBookmarks

This comment has been minimized.

Sign in to view
Copy link
@diracdeltas

diracdeltas Nov 13, 2018

Member

but if that test fails then testCanDeleteHistorySites won't run. i guess it's the same pattern as some other tests though.

This comment has been minimized.

Sign in to view
Copy link
@darkdh

darkdh Nov 13, 2018

Author Member

I just followed the original test flow. It was running preference and history create, list and then delete them all. It was all chained.
I can make each of them independent if you want.
@darkdh darkdh force-pushed the sqs_test branch from 0204f18 to fc28632 Nov 13, 2018
@darkdh
Copy link
Member Author

darkdh commented Nov 13, 2018 

=== npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ merge                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=1.2.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ tape-run [dev]                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ tape-run > browser-run > browser-launcher > merge            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/722                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 1 low severity vulnerability in 4393 scanned packages
  1 vulnerability requires manual review. See the full report for details.
@diracdeltas
Copy link
Member

diracdeltas commented Nov 13, 2018

#232 is the vuln issue
@diracdeltas
diracdeltas reviewed Nov 13, 2018
View changes
@darkdh darkdh force-pushed the sqs_test branch from fc28632 to 23a7240 Nov 16, 2018
@SergeyZhukovsky
Copy link
Member

SergeyZhukovsky commented Nov 16, 2018

++ on the changes. We have to fix the travis error as autodeploy will be broken from that point.
@darkdh
Sort records obtained from SQS to ensure order so we can decode them
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
8396031 
successfully
@darkdh darkdh force-pushed the sqs_test branch 4 times, most recently from 98cf57f to c02bde8 Nov 16, 2018
darkdh added 2 commits Nov 13, 2018
@darkdh
Add SQS test
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
2642155
@darkdh
Prevent infinite recursion when retrieved messages number are bigger …
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
4adbd9e 
…than approximate number of messages
@darkdh darkdh force-pushed the sqs_test branch from c02bde8 to 2642155 Nov 17, 2018
@SergeyZhukovsky
SergeyZhukovsky approved these changes Nov 17, 2018
View changes
Copy link
Member

SergeyZhukovsky left a comment

++
@darkdh darkdh merged commit 6da67f0 into staging Nov 17, 2018
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@darkdh darkdh deleted the sqs_test branch Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SergeyZhukovsky SergeyZhukovsky

@diracdeltas diracdeltas

Assignees

@darkdh darkdh

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

SQS code needs tests
3 participants
@darkdh @diracdeltas @SergeyZhukovsky
You can’t perform that action at this time.