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

Dev/sessiontest #1460

Merged
merged 1 commit into from
Jul 14, 2021
Merged

Dev/sessiontest #1460

merged 1 commit into from
Jul 14, 2021

Conversation

cjiang2000
Copy link
Contributor

added sessionId testing

J=SLAP-1409
TEST=automatic

ran tests and they work as expected.

@coveralls
Copy link

coveralls commented Jul 7, 2021

Coverage Status

Coverage increased (+0.04%) to 59.266% when pulling dc8379a on dev/sessiontest into 3c4b5d3 on develop.

@oshi97
Copy link
Contributor

oshi97 commented Jul 8, 2021

we'll wanna rebase this guy on top of develop so that it doesn't include changes from #1452 (properly implement sessionTrackingEnabled)

"commander": "^2.7.1",
"lodash.get": "^4.0.0",
"lodash.isequal": "^4.0.0",
"validator": "^8.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be causing a build error. There's no , after this line, so npm fails ti parse the package-lock

@@ -3,6 +3,11 @@ import SearchConfig from '../../src/core/models/searchconfig';
import Storage from '../../src/core/storage/storage';
import StorageKeys from '../../src/core/storage/storagekeys';

jest.mock('../../src/core/utils/uuid');
const foo = require('../../src/core/utils/uuid');
Copy link
Contributor

Choose a reason for hiding this comment

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

does importing generateUUID like

const { generateUUID } = require('../../src/core/utils/uuid')

work? If that doesn't work we should rename foo to uuid

@@ -41,6 +46,44 @@ describe('Search requests are created properly', () => {
});
});

describe('Search requests are created properly', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't have two describe blocks with the same name. We'd either put these tests in the block above or give this describe block another name.

);
});

it('sessionId is passed in universal search', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here and below, we're verifying that sessionId is not passed in search

tests/core/core.js Show resolved Hide resolved
@cjiang2000 cjiang2000 merged commit 733ae8d into develop Jul 14, 2021
@cjiang2000 cjiang2000 deleted the dev/sessiontest branch July 15, 2021 14:49
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.

4 participants