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.

Sign up for GitHub

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

Tech/update jest, babel and adapt mocks #1097

Merged
merged 10 commits into from
Jul 14, 2020
Merged

Tech/update jest, babel and adapt mocks #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
284153f
Update jest dependencies
NearW Jul 12, 2020
0ff7a8e
Update babel
NearW Jul 12, 2020
be488b2
Fix warning regarding jscore
NearW Jul 12, 2020
ca9df7b
Update mocks in test
NearW Jul 13, 2020
74df67b
Update simplified mocks of element and window
NearW Jul 13, 2020
c795aa9
Remove deprecated polyfill and use corejs3 instead
NearW Jul 13, 2020
4b32538
Add browserslist to package.json
NearW Jul 13, 2020
cb61f70
Merge branch 'master' into tech/update-jest
BridgeAR Jul 14, 2020
c8d2f00
Refactor simplified promises
NearW Jul 14, 2020
b10fc92
Merge remote-tracking branch 'origin/tech/update-jest' into tech/upda…
NearW Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions visualization/.babelrc
View file
Open in desktop

This file was deleted.

7 changes: 2 additions & 5 deletions visualization/app/codeCharta/ui/attributeSideBar/attributeSideBar.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,8 @@ describe("AttributeSideBarController", () => {
}

function withMockedCodeMapPreRenderService() {
codeMapPreRenderService = attributeSideBarController["codeMapPreRenderService"] = jest.fn<CodeMapPreRenderService>(() => {
return {
getRenderFileMeta: jest.fn().mockReturnValue({ fileName: "my_fileName" })
}
})()
codeMapPreRenderService.getRenderFileMeta = jest.fn().mockReturnValue({ fileName: "my_fileName" })
attributeSideBarController["codeMapPreRenderService"] = codeMapPreRenderService
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to add a linter rule to use the dot notation in the future (if you agree, in a separate PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accessing private fields or functions is quite useful when writing tests or setting up a test.
We could enable that rule for our production code for sure, but tests shouldn't be included.

Copy link
Member

Choose a reason for hiding this comment

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

Using the dot notation is independent from accessing private fields :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How? When I want to access a private field, I have to use class["private_field"]? instead of class.private_field

Copy link
Member

@BridgeAR BridgeAR Jul 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

TIL. I was not aware that TypeScript used this specific notation to access private fields. IMO it's a bad practice to access any private field. I do not want to go into that discussion here though. I would actively mark access like these (by adding a comment) that it's a private properly that it's intentionally accessed and that it's the TypeScript way to do so.

Calling something a private field that is still accessible is somewhat weird. The reason that it's working like that was AFAIK that it was difficult to create actual private members when TypeScript came up. That changed in the meanwhile and now JS has actual private fields that do not allow any access from outside.

I personally always test internals by either: using the dependency injection pattern or by just plainly checking the overall output of a higher level API that should only work if the internal path is hit. If the coverage does not show the expected lines to be covered, something is fishy.

To get back to the actual comment: for now it would be possible to exclude tests for such a rule. On the long term, I'd try to refactor the code to the dependency injection pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could use the typescript-es-lint rule. It has an option to allow private class member access like this.

I'm not sure if you can allow/disallow individual rules. I think you can just define a pattern that applies to all rules.

Copy link
Member

Choose a reason for hiding this comment

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

It is possible to enable specific rules for specific files by using overrides. It should be sufficient to just use the "native" rule.

}

describe("constructor", () => {
Expand Down
15 changes: 2 additions & 13 deletions visualization/app/codeCharta/ui/codeMap/codeMap.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ describe("ColorSettingsPanelController", () => {

beforeEach(() => {
restartSystem()
mockElement()
rebuildController()
mockElement()
})

function restartSystem() {
Expand All @@ -31,18 +31,7 @@ describe("ColorSettingsPanelController", () => {
}

function mockElement() {
$element = jest.fn<Element>(() => {
return [
{
children: [
{
clientWidth: 50,
clientHeight: 100
}
]
}
]
})()
$element = [{ children: [{ clientWidth: 50, clientHeight: 100 }] }] as any
}

function rebuildController() {
Expand Down
32 changes: 10 additions & 22 deletions visualization/app/codeCharta/ui/codeMap/codeMap.label.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,17 @@ describe("CodeMapLabelService", () => {
}

function withMockedThreeCameraService() {
threeCameraService = jest.fn<ThreeCameraService>(() => {
return {
camera: {
position: {
distanceTo: jest.fn()
}
}
}
})()
threeCameraService.camera = { position: { distanceTo: jest.fn() } }
}

function withMockedThreeSceneService() {
threeSceneService = jest.fn<ThreeSceneService>(() => {
return {
mapGeometry: new THREE.Mesh(new THREE.BoxGeometry(10, 10, 10)),
labels: {
add: jest.fn(),
children: jest.fn()
}
Object.assign(threeSceneService, {
mapGeometry: new THREE.Mesh(new THREE.BoxGeometry(10, 10, 10)),
labels: {
add: jest.fn(),
children: jest.fn()
}
})()
})
}

function setCanvasRenderSettings() {
Expand Down Expand Up @@ -96,11 +86,9 @@ describe("CodeMapLabelService", () => {

createElementOrigin = document.createElement

document.createElement = jest.fn(() => {
return {
getContext: () => {
return canvasCtxMock
}
document.createElement = jest.fn().mockReturnValue({
getContext: () => {
return canvasCtxMock
}
})

Expand Down
26 changes: 8 additions & 18 deletions visualization/app/codeCharta/ui/codeMap/threeViewer/threeOrbitControlsService.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ describe("ThreeOrbitControlsService", () => {

let vector: Vector3

afterEach(() => {
jest.resetAllMocks()
})

beforeEach(() => {
restartSystem()
rebuildService()
Expand All @@ -49,23 +45,17 @@ describe("ThreeOrbitControlsService", () => {
function withMockedThreeCameraService() {
const camera = new PerspectiveCamera(100, 0, 0, 0)
camera.position.set(vector.x, vector.y, vector.z)
threeCameraService = threeOrbitControlsService["threeCameraService"] = jest.fn<ThreeCameraService>(() => {
return {
camera
}
})()

threeCameraService.camera = camera
threeOrbitControlsService["threeCameraService"] = threeCameraService
}

function withMockedThreeSceneService() {
threeSceneService = threeOrbitControlsService["threeSceneService"] = jest.fn<ThreeSceneService>(() => {
return {
scene: {
add: jest.fn(),
remove: jest.fn()
},
mapGeometry: new THREE.Mesh(new THREE.BoxGeometry(10, 10, 10))
}
})()
threeSceneService.scene.add = jest.fn()
threeSceneService.scene.remove = jest.fn()
threeSceneService.mapGeometry = new THREE.Mesh(new THREE.BoxGeometry(10, 10, 10))

threeOrbitControlsService["threeSceneService"] = threeSceneService
}

function withMockedControlService() {
Expand Down
8 changes: 2 additions & 6 deletions visualization/app/codeCharta/ui/dialog/dialog.download.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,8 @@ describe("DialogDownloadController", () => {
}

function withMockedCodeMapPreRenderService() {
codeMapPreRenderService = jest.fn<CodeMapPreRenderService>(() => {
return {
getRenderMap: jest.fn().mockReturnValue(VALID_NODE_WITH_PATH_AND_EXTENSION),
getRenderFileMeta: jest.fn().mockReturnValue(FILE_META)
}
})()
codeMapPreRenderService.getRenderMap = jest.fn().mockReturnValue(VALID_NODE_WITH_PATH_AND_EXTENSION)
codeMapPreRenderService.getRenderFileMeta = jest.fn().mockReturnValue(FILE_META)
}

function getFilteredFileContent(name: DownloadCheckboxNames) {
Expand Down
7 changes: 2 additions & 5 deletions visualization/app/codeCharta/ui/edgeChooser/edgeChooser.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,8 @@ describe("EdgeChooserController", () => {
}

function withMockedCodeMapActionsService() {
codeMapActionsService = edgeChooserController["codeMapActionsService"] = jest.fn<CodeMapActionsService>(() => {
return {
updateEdgePreviews: jest.fn()
}
})()
codeMapActionsService.updateEdgePreviews = jest.fn()
edgeChooserController["codeMapActionsService"] = codeMapActionsService
}

describe("constructor", () => {
Expand Down
14 changes: 4 additions & 10 deletions visualization/app/codeCharta/ui/edgeSettingsPanel/edgeSettingsPanel.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,13 @@ describe("EdgeSettingsPanelController", () => {
}

function withMockedEdgeMetricDataService(amountOfAffectedBuildings = 0) {
edgeMetricDataService = edgeSettingsPanelController["edgeMetricDataService"] = jest.fn<EdgeMetricDataService>(() => {
return {
getAmountOfAffectedBuildings: jest.fn().mockReturnValue(amountOfAffectedBuildings)
}
})()
edgeMetricDataService.getAmountOfAffectedBuildings = jest.fn().mockReturnValue(amountOfAffectedBuildings)
edgeSettingsPanelController["edgeMetricDataService"] = edgeMetricDataService
}

function withMockedCodeMapActionsService() {
codeMapActionsService = edgeSettingsPanelController["codeMapActionsService"] = jest.fn<CodeMapActionsService>(() => {
return {
updateEdgePreviews: jest.fn()
}
})()
codeMapActionsService.updateEdgePreviews = jest.fn()
edgeSettingsPanelController["codeMapActionsService"] = codeMapActionsService
}

describe("constructor", () => {
Expand Down
9 changes: 3 additions & 6 deletions visualization/app/codeCharta/ui/mapTreeView/mapTreeView.level.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ describe("MapTreeViewLevelController", () => {

beforeEach(() => {
restartSystem()
withMockedCodeMapPreRenderService()
rebuildController()
withMockedCodeMapPreRenderService()
withMockedEventMethods($rootScope)
})

Expand All @@ -52,11 +52,8 @@ describe("MapTreeViewLevelController", () => {
}

function withMockedCodeMapPreRenderService() {
codeMapPreRenderService = jest.fn<CodeMapPreRenderService>(() => {
return {
getRenderMap: jest.fn().mockReturnValue(VALID_NODE_WITH_ROOT_UNARY)
}
})()
codeMapPreRenderService.getRenderMap = jest.fn().mockReturnValue(VALID_NODE_WITH_ROOT_UNARY)
mapTreeViewLevelController["codeMapPreRenderService"] = codeMapPreRenderService
}

describe("onBuildingHovered", () => {
Expand Down
46 changes: 8 additions & 38 deletions visualization/app/codeCharta/ui/nodeContextMenu/nodeContextMenu.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ describe("nodeContextMenuController", () => {
restartSystem()
mockElement()
mockWindow()
rebuildController()
withMockedCodeMapActionService()
withMockedCodeMapPreRenderService()
rebuildController()
withMockedHideNodeContextMenuMethod()

NodeDecorator.preDecorateFile(TEST_DELTA_MAP_A)
Expand All @@ -51,27 +51,11 @@ describe("nodeContextMenuController", () => {
}

function mockElement() {
element = jest.fn<Element>(() => {
return [
{
children: [
{
clientWidth: 50,
clientHeight: 100
}
]
}
]
})()
element = [{ children: [{ clientWidth: 50, clientHeight: 100 }] }] as any
}

function mockWindow() {
$window = jest.fn<IWindowService>(() => {
return {
innerWidth: 800,
innerHeight: 600
}
})()
$window = { innerWidth: 800, innerHeight: 600 } as IWindowService
}

function rebuildController() {
Expand All @@ -87,34 +71,20 @@ describe("nodeContextMenuController", () => {
}

function withMockedCodeMapActionService() {
codeMapActionsService = nodeContextMenuController["codeMapActionsService"] = jest.fn<CodeMapActionsService>(() => {
return {
getParentMP: jest.fn(),
anyEdgeIsVisible: jest.fn(),
markFolder: jest.fn(),
unmarkFolder: jest.fn()
}
})()
codeMapActionsService.getParentMP = jest.fn()
codeMapActionsService["anyEdgeIsVisible"] = jest.fn()
codeMapActionsService.markFolder = jest.fn()
codeMapActionsService.unmarkFolder = jest.fn()
}

function withMockedCodeMapPreRenderService() {
codeMapPreRenderService = nodeContextMenuController["codeMapPreRenderService"] = jest.fn<CodeMapPreRenderService>(() => {
return {
getRenderMap: jest.fn(() => {
return TEST_DELTA_MAP_A.map
})
}
})()
codeMapPreRenderService.getRenderMap = jest.fn().mockReturnValue(TEST_DELTA_MAP_A.map)
}

function withMockedHideNodeContextMenuMethod() {
nodeContextMenuController.hideNodeContextMenu = jest.fn()
}

afterEach(() => {
jest.resetAllMocks()
})

describe("constructor", () => {
it("should subscribe to 'show-node-context-menu' events", () => {
NodeContextMenuController.subscribeToShowNodeContextMenu = jest.fn()
Expand Down
Loading