-
Notifications
You must be signed in to change notification settings - Fork 0
Add backbeat route apis #4
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
5305e7d to
670da80
Compare
51e8e9c to
3d0f040
Compare
3d0f040 to
e09c5a5
Compare
d7986fe to
5b20462
Compare
04d210b to
b346637
Compare
b346637 to
a3b3666
Compare
.github/workflows/test.yml
Outdated
| - name: Stop Cloudserver | ||
| if: always() | ||
| run: docker compose -f .github/docker-compose.cloudserver-mongo.yml down | ||
| run: docker compose -f .github/docker-compose.yml --env-file .github/docker.env --profile mongo down |
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.
is this needed? the runner will be destroyed anyway...
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 still applies @SylvainSenechal
| import { describeForBackbeatSetup } from './testHelpers'; | ||
| import assert from 'assert'; | ||
|
|
||
| // Note : these tests are relatively simple, as it's not straightforward to setup |
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.
note for thought (nothing to change now!) : instead of deploying "full" services (and thus having some sort of integration tests), we could have "mocked" the API in the tests (kind of more "unit test" approach, which avoids depending on the external packages & setting up all services)
00b92a5 to
18472a8
Compare
.github/docker-compose.backbeat.yml
Outdated
| start_period: 10s | ||
|
|
||
| mongodb-init: | ||
| image: mongo:4.4 |
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.
Why are we using the 4.4 ?
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.
the higher ones aren't working on macs, I think its okish to keep it like this here
.github/workflows/test.yml
Outdated
| - name: Stop Cloudserver | ||
| if: always() | ||
| run: docker compose -f .github/docker-compose.cloudserver-metadata.yml down | ||
| run: docker compose -f .github/docker-compose.yml --env-file .github/docker.env --profile metadata down |
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.
.github/workflows/test.yml
Outdated
|
|
||
| - name: Stop Backbeat and Cloudserver | ||
| if: always() | ||
| run: docker compose -f .github/docker-compose.yml --env-file .github/docker.env --profile backbeat down |
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.
| Size: Integer, | ||
|
|
||
| LastModified: Timestamp | ||
| } No newline at end of file |
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.
Missing line at EOF
| const input: GetLocationsIngestionStatusCommandInput = {}; | ||
| const cmd = new GetLocationsIngestionStatusCommand(input); | ||
| const result = await proxyBackbeatApisClient.send(cmd); | ||
| assert.strictEqual(result.$metadata.httpStatusCode, 200); |
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.
can't we add another assertion for this test and the others after ( not blocking but just to make sure we're receiving the intended infos )
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.
There isn't much data unfortunately for these as ingestion isn't setup, I will use this new client with the zenko tests to also test it
4cda2cb to
b3e03bf
Compare
|
/approve |
|
I have successfully merged the changeset of this pull request
Please check the status of the associated issue CLDSRVCLT-5. Goodbye sylvainsenechal. The following options are set: approve |
ISSUE: CLDSRVCLT-5