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

EventStream Puzzler #75

Open
wants to merge 6 commits into
base: sarah-eventstream-puzzler
Choose a base branch
from

Conversation

reimannsarah
Copy link

@reimannsarah reimannsarah commented Aug 24, 2023

What this is
Red phase tests for eventStream puzzler

Ticket
Sarah-EventStream-Puzzler

Steps to Verify
run yarn test in terminal

Media
Screenshot 2023-08-25 at 4 29 38 PM

src/index.ts Outdated

export function scoreEventStream(eventStream: ReadonlyArray<event>): highest_score {
return {
events: [],
Copy link
Author

Choose a reason for hiding this comment

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

I added a return object here because otherwise my test suite would not run and I have typed the output for the function

Comment on lines 5 to 15
it('should return an object with an events property', () => {
const actual = scoreEventStream(eventStream);

expect(actual).toHaveProperty("events");
})

it('should return an object with a score property', () => {
const actual = scoreEventStream(eventStream);

expect(actual).toHaveProperty("score");
})
Copy link

Choose a reason for hiding this comment

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

these two tests can be combined, since you're testing that the return object has the expected properties.

just do both expects in the same test.

expect(actual).toHaveProperty("score");
})

it('successfully assess the length of the inputted array', () => {
Copy link

Choose a reason for hiding this comment

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

future tests should include checks for different region lengths? If not, just add a comment here saying that the default length is 5.

src/index.ts Outdated
@@ -1 +1,16 @@
//Define class/functions here
type event = {
Copy link

Choose a reason for hiding this comment

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

interface

src/index.ts Outdated
eventType: string
}

type highest_score = {
Copy link

Choose a reason for hiding this comment

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

interface

Copy link

Choose a reason for hiding this comment

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

high_score -> HighScore

@reimannsarah reimannsarah changed the title write failing tests EventStream Puzzler red phase Aug 25, 2023
@reimannsarah reimannsarah changed the title EventStream Puzzler red phase EventStream Puzzler Aug 25, 2023
expect(actual).toHaveProperty("score");
})

it('length of events array in return object should be no greater than 5', () => {
Copy link

Choose a reason for hiding this comment

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

since the function now accepts the regionLength argument, then we need additional tests to verify expected array lengths. Default vs > vs <

@@ -0,0 +1,13 @@
export interface event {
Copy link

Choose a reason for hiding this comment

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

Suggested change
export interface event {
export interface Event {

Copy link

Choose a reason for hiding this comment

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

interfaces and types should be capialized.

Copy link

Choose a reason for hiding this comment

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

type EventStream = ReadonlyArray<Event>;

return eventStreamWithScores.map(e => { return e.score })
}

export function scoreEventStream(eventStream: ReadonlyArray<event>, subRegionLength: number = 5) {
Copy link

Choose a reason for hiding this comment

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

const DEFAULT_SUBREGION_LENGTH = 5

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.

2 participants