Grid UI Unit Tests #8865

raju249 · 2020-11-11T10:07:20Z

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have read the contributing document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

AutomatedTester · 2020-11-11T11:34:58Z

javascript/grid-ui/jest.config.js

+    "<rootDir>/src"
+  ],
+  testMatch: [
+    "<rootDir>/src/tests/*.test.ts",


Suggested change

"<rootDir>/src/tests/*.test.ts",

"<rootDir>/src/tests/*.test.ts",

"<rootDir>/src/tests/*.test.tsx",

AutomatedTester · 2020-11-11T11:35:31Z

javascript/grid-ui/src/components/CheckBox.ts

@@ -0,0 +1,30 @@
+import * as React from 'react';


rename this file so that it ends with tsx instead of ts

AutomatedTester · 2020-11-11T11:35:39Z

javascript/grid-ui/src/tests/Checkbox.test.ts

@@ -0,0 +1,15 @@
+import * as React from 'react';


rename this file so that it ends with tsx instead of ts

AutomatedTester · 2020-11-11T11:39:19Z

javascript/grid-ui/src/components/CheckBox.ts

+}, {
+    isChecked: boolean
+  }> {
+  constructor(props) {


Suggested change

constructor(props) {

constructor(props: any) {

AutomatedTester · 2020-11-20T09:15:32Z

javascript/grid-ui/package.json

@@ -61,5 +61,19 @@
 			"last 1 firefox version",
 			"last 1 safari version"
 		]
+	},
+	"devDependencies": {


All of these need to be at the to be in the top level package.json file in the root of the repo

AutomatedTester · 2020-11-20T09:16:31Z

javascript/grid-ui/BUILD.bazel

+        "jest.config.js"
+    ],
+    data = [
+        "//:jest.config.js"


By having // you're telling it to look at the root of the repo which means it will error.

AutomatedTester · 2020-11-20T09:38:20Z

javascript/grid-ui/BUILD.bazel

+        "--ci",
+        "--colors",
+        "--config",
+        "jest.config.js"


Suggested change

"jest.config.js"

"$(location jest.config.js)"

That will find it local to the bazel build

AutomatedTester · 2020-11-23T14:29:33Z

javascript/grid-ui/src/setupTests.ts

@@ -0,0 +1,3 @@
+import { configure } from 'enzyme';


This file should probably be a .tsx file so we don't get that weird issue we had before

AutomatedTester · 2020-11-23T14:30:18Z

jest.config.js

@@ -0,0 +1,19 @@
+module.exports = {


We don't need this file, we can use the one that is in the grid-ui directory

AutomatedTester · 2020-11-23T14:31:36Z

javascript/grid-ui/src/App.tsx

@@ -21,6 +22,7 @@ import NodeType from "./models/node";
 const cache = new InMemoryCache();
 const link = new HttpLink({
 	uri: GridConfig.serverUri,
+  fetch


Is this line needed? It feels like this would error since it's not in an object format

Yes, its needed.

There are some tests that fail with the following message:

Test suite failed to run Invariant Violation: fetch is not found globally and no fetcher passed, to fix pass a fetch for your environment like https://www.npmjs.com/package/unfetch. For example: import fetch from 'unfetch'; import { createHttpLink } from 'apollo-link-http'; const link = createHttpLink({ uri: '/graphql', fetch: fetch }); 20 | 21 | const cache = new InMemoryCache(); > 22 | const link = new HttpLink({ | ^ 23 | uri: GridConfig.serverUri, 24 | }); 25 | at new InvariantError (../../../../../../../../../node_modules/ts-invariant/lib/invariant.esm.js:12:28) at Object.<anonymous>.exports.checkFetcher (../../../../../../../../../node_modules/apollo-link-http-common/src/index.ts:178:11) at Object.<anonymous>.exports.createHttpLink (../../../../../../../../../node_modules/apollo-link-http/src/httpLink.ts:46:3) at new HttpLink (../../../../../../../../../node_modules/apollo-link-http/src/httpLink.ts:259:11) at Object.<anonymous> (../../../../src/App.tsx:22:14) at Object.<anonymous> (../../../../src/core/Status.ts:2:1) at Object.<anonymous> (../../../../src/screens/Console/Console.tsx:14:1) at Object.<anonymous> (../../../../src/screens/Console/Console.keybinds.ts:4:1) at Object.<anonymous> (../../../../src/components/KeyBoard/KeyBoardHelp.tsx:7:1) at Object.<anonymous> (../../../../src/components/NavBar/NavBar.tsx:12:1) at Object.<anonymous> (../../../../src/tests/components/NavBar.test.tsx:3:1)

When checked on SO, that was the fix that was suggested on multiple questions of similar error stack trace.

I am able to build and start the server. UI page opens fine.

Ok, can we have it as a key/value pair then

AutomatedTester · 2020-11-23T14:32:22Z

javascript/grid-ui/BUILD.bazel

+        "--ci",
+        "--colors",
+        "--config",
+        "$(rootpath jest.config.js)"


Suggested change

"$(rootpath jest.config.js)"

"$(location jest.config.js)"

javascript/grid-ui/BUILD.bazel

Co-authored-by: David Burns <david.burns@theautomatedtester.co.uk>

AutomatedTester reviewed Nov 11, 2020

View reviewed changes

raju249 marked this pull request as draft November 12, 2020 03:37

raju249 force-pushed the grid-ui-unit-tests branch from 6504624 to de70765 Compare November 18, 2020 06:36

AutomatedTester reviewed Nov 20, 2020

View reviewed changes

raju249 force-pushed the grid-ui-unit-tests branch from 2f66ac0 to 4ed5897 Compare November 23, 2020 04:23

raju249 marked this pull request as ready for review November 23, 2020 04:23

AutomatedTester requested changes Nov 23, 2020

View reviewed changes

raju249 added 2 commits November 24, 2020 10:29

Fix path in jest config

e69e982

Fix review comments, remove unused jest config

3c0535e

raju249 force-pushed the grid-ui-unit-tests branch from 4ed5897 to 3c0535e Compare November 24, 2020 05:42

raju249 requested a review from AutomatedTester November 24, 2020 05:43

AutomatedTester reviewed Nov 24, 2020

View reviewed changes

javascript/grid-ui/BUILD.bazel Show resolved Hide resolved

Add key/val for fetch and small in bazel test target

f71c07b

raju249 requested a review from AutomatedTester November 25, 2020 04:12

AutomatedTester approved these changes Nov 25, 2020

View reviewed changes

Merge branch 'trunk' into grid-ui-unit-tests

82eb4fe

AutomatedTester merged commit 78eca82 into SeleniumHQ:trunk Nov 25, 2020

pujagani pushed a commit to pujagani/selenium that referenced this pull request Nov 26, 2020

Grid UI Unit Tests (SeleniumHQ#8865)

8778f28

Co-authored-by: David Burns <david.burns@theautomatedtester.co.uk>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grid UI Unit Tests #8865

Grid UI Unit Tests #8865

raju249 commented Nov 11, 2020

AutomatedTester Nov 11, 2020

AutomatedTester Nov 11, 2020

AutomatedTester Nov 11, 2020

AutomatedTester Nov 11, 2020

AutomatedTester Nov 20, 2020

AutomatedTester Nov 20, 2020

AutomatedTester Nov 20, 2020

AutomatedTester Nov 23, 2020

AutomatedTester Nov 23, 2020

AutomatedTester Nov 23, 2020

raju249 Nov 24, 2020

raju249 Nov 24, 2020

AutomatedTester Nov 24, 2020

AutomatedTester Nov 23, 2020

	"<rootDir>/src/tests/*.test.ts",
	"<rootDir>/src/tests/*.test.ts",
	"<rootDir>/src/tests/*.test.tsx",

Grid UI Unit Tests #8865

Grid UI Unit Tests #8865

Conversation

raju249 commented Nov 11, 2020

Description

Motivation and Context

Types of changes

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment