Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

Conversation

@tbarlow12
Copy link
Contributor

@tbarlow12 tbarlow12 commented Jun 25, 2019

Discovered this small bug in the test. Overloading the variable name masterKey, using result instead.

Copy link
Contributor

@mydiemho mydiemho left a comment

Choose a reason for hiding this comment

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

lgtm, just some suggestions

expect(result).toBeNull();
});

it("gets master key", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: rename test description to start with should/should not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

const service = createService();
const masterKey = await service.getMasterKey();
expect(masterKey).toEqual(masterKey);
const result = await service.getMasterKey();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would call this actualMasterKey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

@tbarlow12 tbarlow12 merged commit 83cf9e4 into dev Jun 25, 2019
@tbarlow12 tbarlow12 deleted the tabarlow/test-patch branch July 12, 2019 16:56
tbarlow12 added a commit that referenced this pull request Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants