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

Bugfix/313/invalid session #323

Merged
merged 16 commits into from
Jan 28, 2020
Merged

Bugfix/313/invalid session #323

merged 16 commits into from
Jan 28, 2020

Conversation

svettwer
Copy link
Contributor

This PR is based on the changes of #319. I'll rebase the PR in case #319 is merged earlier.

@cliffle cliffle self-assigned this Jan 28, 2020
@svettwer svettwer changed the base branch from bugfix/314/getDriver-driver-from-undefined to develop January 28, 2020 09:49
Copy link
Contributor

@cliffle cliffle left a comment

Choose a reason for hiding this comment

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

When I run the tests, I noticed that the following message appears in the output:

jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/context/__mocks__/index.ts
    * <rootDir>/context/sahi/__mocks__/index.ts

I also want to note, that you added comments like // noinspection HtmlRequiredAltAttribute,HtmlDeprecatedAttribute , which disables the warnings in IntelliJ. This does not affect VS Code users as they don't have a html inspection like in IntelliJ. Do we want those lines in our code?

Comment on lines 106 to 111
const webDriverMock = createWebDriverMock(jest.fn()
.mockResolvedValueOnce("<html></html>")
.mockResolvedValueOnce("<html><div></div></html>")
.mockResolvedValueOnce("<html></html>")
.mockResolvedValueOnce("<html><div></div></html>")
.mockResolvedValueOnce("<html></html>"));
Copy link
Contributor

Choose a reason for hiding this comment

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

This webDriverMock is also used in should not wait longer than the timeout defines. I think we can refactor this into a function.

Comment on lines 73 to 75
const webDriverMock = createWebDriverMock(jest.fn()
.mockResolvedValueOnce("<html></html>")
.mockResolvedValue("<html><div></div></html>"));
Copy link
Contributor

Choose a reason for hiding this comment

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

This webDriverMock was also used in should wait if page is changing its dom over time. I think we can refactor this into a function.

@cliffle cliffle assigned svettwer and unassigned cliffle Jan 28, 2020
@svettwer
Copy link
Contributor Author

svettwer commented Jan 28, 2020

Hiho! 👋

Thx for the review. The duplicate was a good catch! 👍
I applied the changes accordingly and removed the Intellij specific comments.

BR,
Sven

@svettwer svettwer assigned cliffle and unassigned svettwer Jan 28, 2020
@svettwer
Copy link
Contributor Author

When I run the tests, I noticed that the following message appears in the output:

jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/context/__mocks__/index.ts
    * <rootDir>/context/sahi/__mocks__/index.ts

Concerning this, I'm not sure where this comes from. I checked the implementation and could not find a duplicated manual mock... 🤔
Have you found it?

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@cliffle cliffle assigned svettwer and unassigned cliffle Jan 28, 2020
@cliffle
Copy link
Contributor

cliffle commented Jan 28, 2020

When I run the tests, I noticed that the following message appears in the output:

jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/context/__mocks__/index.ts
    * <rootDir>/context/sahi/__mocks__/index.ts

Concerning this, I'm not sure where this comes from. I checked the implementation and could not find a duplicated manual mock...
Have you found it?

I think it's jestjs/jest#2070

@svettwer
Copy link
Contributor Author

Okay, then I'd recommend to ignore it. As the PR in the issue is merged since August 19 it might be fixed when we update jest.

@svettwer svettwer merged commit b8bc78b into develop Jan 28, 2020
@svettwer svettwer deleted the bugfix/313/invalid-session-id branch January 28, 2020 18:26
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