feat(tests): refactor sinkMain and create tests
#897
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asharonbaltazar
had a problem deploying
to
refactor-tests-sink
GitHub Actions
Failure
asharonbaltazar
had a problem deploying
to
refactor-tests-sink
GitHub Actions
Failure
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
force-pushed
the
refactor-tests-sink
branch
from
8bd3057 to
50b681c
Compare
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink-app
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink-kibana
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink-kibana
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink-app
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
force-pushed
the
refactor-tests-sink
branch
from
1e7ba2c to
52525cd
Compare
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
refactor-tests-sink
GitHub Actions
Inactive
tbolt
reviewed
Dec 9, 2024
| @@ -13,18 +13,9 @@ | |||
| }, | |||
There was a problem hiding this comment.
This is great, I assumed we were using most of these packages.
There was a problem hiding this comment.
Agreed, this is some great cleanup
| @@ -1,14 +1,8 @@ | |||
| import { Handler } from "aws-lambda"; | |||
| import { KafkaRecord, opensearch } from "shared-types"; | |||
| import { KafkaEvent } from "shared-types"; | |||
There was a problem hiding this comment.
combine this with the above import?
| await handler( | ||
| { | ||
| eventSource: "SelfManagedKafka", | ||
| bootstrapServers: "b-1.master-msk.zf7e0q.c7.kafka.us-east-1.amazonaws.com:9094", |
There was a problem hiding this comment.
are these real endpoints? lets take these out if they're not needed
cc: @13bfrancis
| @@ -0,0 +1,87 @@ | |||
| import { describe, it, expect, vi, afterEach } from "vitest"; | |||
There was a problem hiding this comment.
I'm wondering if we could use a function for the handler setup. Something like-
function createHandlerEvent(record) {
return {
eventSource: "SelfManagedKafka",
bootstrapServers: "kafka",
records: { [`"${record}"`]: [] },
};
}
and then in the tests-
await handler(createHandlerEvent("aws.onemac.migration.cdc-0"), expect.anything(), vi.fn());
There was a problem hiding this comment.
Overall I think this looks great. Some great refactoring to make it more testable, great removal of unused code and dependencies, and good tests. Like Tyler, I'll likely defer to someone with more familiarity with this part of the app for final approval, but this looks like great work to me.
| kafkaRecords: KafkaRecord[], | ||
| topicPartition: string, | ||
| ) => { | ||
| const officers = async (kafkaRecords: KafkaRecord[], topicPartition: string) => { |
There was a problem hiding this comment.
Just curious, where does this const get its name from? Is "officers" a Kafka term, or does it have a OneMAC-specific meaning?
| @@ -82,17 +82,45 @@ const prettyPrintJsonInObject = (obj: any): any => { | |||
| return obj; | |||
| }; | |||
|
|
|||
| /** | |||
| * Returns the `osDomain` and `indexNamespace` env variables. Passing `baseIndex` appends the arg to the `index` variable | |||
There was a problem hiding this comment.
Loving this documentation!
| @@ -13,18 +13,9 @@ | |||
| }, | |||
There was a problem hiding this comment.
Agreed, this is some great cleanup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Improve
sinkMainlogic and write testsLinked Issues to Close
https://jiraent.cms.gov/browse/OY2-31425
Approach