-
Notifications
You must be signed in to change notification settings - Fork 478
[Feature] RecordScanner client #1100
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
Conversation
5114141
to
0001cae
Compare
@alexpitsikoulis I'll review this in a bit, basically though, as many tests as you can build proving it against a real RSS service with it, the better. |
0001cae
to
2d1f53d
Compare
2d1f53d
to
462eb89
Compare
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.
Overall really great job here Alex, the tests are good. Left some comments, but they're mostly small changes.
What we want to do prior to folding this in is test it against an actual service.
Can you write up another test file that tests it against the service? We can store all the test data related to that in envars for ci.
* Create domain separator in function in wasm * Give domain separator camelCase name * Add Record Scanner domain separator to SDK constants
… each time a client spins up
2621182
to
6c2d52a
Compare
6caffb8
to
604a207
Compare
Excellent work @alexpitsikoulis this is set to merge. |
Motivation
The
RecordScanner
class which implements theRecordProvider
interface serves as a means of access for the Record Scanner Service.Test Plan
Unit tests on initialization of the RecordScanner as well as all of its methods, ensuring that it is making the correct HTTP requests with the correct body and headers and returning the data received from these requests as expected.
Related PRs
Upgrade to
RecordProvider
interface