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

Implement e2e test flow, fix small bugs #65

Merged
merged 4 commits into from
Oct 2, 2023
Merged

Implement e2e test flow, fix small bugs #65

merged 4 commits into from
Oct 2, 2023

Conversation

Siegrift
Copy link
Collaborator

@Siegrift Siegrift commented Oct 2, 2023

Closes #52

@Siegrift Siegrift requested a review from andreogle October 2, 2023 08:53
@Siegrift Siegrift self-assigned this Oct 2, 2023
@@ -14,7 +14,7 @@ export const startServer = (config: Config) => {
const result = await batchInsertData(req.body);
res.status(result.statusCode).header(result.headers).send(result.body);

logger.info('Responded to request "POST /"', result);
logger.debug('Responded to request "POST /"', result);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was too spammy. Note, that I still left the receive log as INFO level.

@@ -1,7 +1,9 @@
# common

Utilities commonly used by other packages. Each common utility lives in its own folder together with its documentation.
The implementation is re-exported by main entry of this package. The package consists of:
> Utilities commonly used by other packages.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To make it consistent with other packages.

debug: (message, context) => logger.debug(message, { context }),
info: (message, context) => logger.info(message, { context }),
warn: (message, context) => logger.warn(message, { context }),
debug: (message, context) => logger.debug(message, context ? { context } : undefined),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you do not pass context, the logs previously had {context: undefined} which is unnecessary (and confusing).

## Getting started

1. If you are using Docker Desktop, you need to change the URL in `pusher/secrets.env` from `localhost` to
`host.docker.internal`, because pusher is running inside a Docker container.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the configurations are not gitignored it will modify source tree, but I don't think we need to care much.

# Secrets must NOT be quoted.
#
# Use "host.docker.internal" if using Docker for Desktop.
HOST_IP=localhost
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interpolated in config to make it easier to work with.

@@ -9,7 +9,7 @@ import { TemplateResponse } from '../sign-template-data';
export const callApi = async (payload: node.ApiCallPayload) => {
logger.debug('Preprocessing API call payload', pick(payload.aggregatedApiCall, ['endpointName', 'oisTitle']));
const processedPayload = await preProcessApiSpecifications(payload);
logger.debug('Performing API call', { processedPayload: processedPayload });
logger.debug('Performing API call', pick(processedPayload.aggregatedApiCall, ['endpointName', 'oisTitle']));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was too spammy.

@@ -50,8 +50,11 @@ export const makeTemplateRequests = async (signedApiUpdate: SignedApiUpdate): Pr
: callApi(operationPayload));

if (node.api.isPerformApiCallFailure(apiCallResponse)) {
const message = `Failed to make API call for the endpoint [${endpoint.oisTitle}] ${endpoint.endpointName}.`;
logger.warn(message, { operationTemplateId });
logger.warn(`Failed to make API call`, {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was missing error message.

@@ -50,7 +50,11 @@ export const postSignedApiData = async (group: SignedApiNameUpdateDelayGroup) =>
logger.warn(
`Failed to make update signed API request.`,
// See: https://axios-http.com/docs/handling_errors
{ ...logContext, error: goAxiosRequest.error.response ?? goAxiosRequest.error }
{
Copy link
Collaborator Author

@Siegrift Siegrift Oct 2, 2023

Choose a reason for hiding this comment

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

Was too spammy and also didn't have proper error message in certain network failures.

@Siegrift Siegrift force-pushed the e2e branch 2 times, most recently from 12919eb to 59cb592 Compare October 2, 2023 09:18
@Siegrift Siegrift merged commit f21927e into main Oct 2, 2023
4 checks passed
@Siegrift Siegrift deleted the e2e branch October 2, 2023 10:23
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.

Implement integration tests
1 participant