From 3545d3d097a7253fb1294683f82ce27afcfcbde2 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@media.mit.edu>
Date: Tue, 20 Nov 2018 00:14:10 -0500
Subject: [PATCH] Refactor shouldDisconnect logic and add more disconnection
unit tests.
---
src/lib/cloud-manager-hoc.jsx | 8 +--
test/unit/util/cloud-manager-hoc.test.jsx | 73 +++++++++++++++++++----
2 files changed, 67 insertions(+), 14 deletions(-)
diff --git a/src/lib/cloud-manager-hoc.jsx b/src/lib/cloud-manager-hoc.jsx
index 7e33cbf9f..6ca6b7500 100644
--- a/src/lib/cloud-manager-hoc.jsx
+++ b/src/lib/cloud-manager-hoc.jsx
@@ -34,7 +34,7 @@ const cloudManagerHOC = function (WrappedComponent) {
this.connectToCloud();
}
- if (this.shouldDisconnect(this.props) && !this.shouldDisconnect(prevProps)) {
+ if (this.shouldDisconnect(this.props, prevProps)) {
this.disconnectFromCloud();
}
}
@@ -49,12 +49,12 @@ const cloudManagerHOC = function (WrappedComponent) {
shouldConnect (props) {
return !this.isConnected() && this.canUseCloud(props) && props.isShowingWithId;
}
- shouldDisconnect (props) {
+ shouldDisconnect (props, prevProps) {
return this.isConnected() &&
( // Can no longer use cloud or cloud provider info is now stale
!this.canUseCloud(this.props) ||
- (this.cloudProvider.projectId !== props.projectId) ||
- (this.cloudProvider.username !== props.username)
+ (props.projectId !== prevProps.projectId) ||
+ (props.username !== prevProps.username)
);
// TODO need to add provisions for viewing someone
// else's project in editor mode
diff --git a/test/unit/util/cloud-manager-hoc.test.jsx b/test/unit/util/cloud-manager-hoc.test.jsx
index d112c0392..7414fc7cb 100644
--- a/test/unit/util/cloud-manager-hoc.test.jsx
+++ b/test/unit/util/cloud-manager-hoc.test.jsx
@@ -6,7 +6,13 @@ import {mount} from 'enzyme';
import VM from 'scratch-vm';
import {LoadingState} from '../../../src/reducers/project-state';
import CloudProvider from '../../../src/lib/cloud-provider';
-jest.mock('../../../src/lib/cloud-provider');
+const mockCloudProviderInstance = {
+ connection: true,
+ requestCloseConnection: jest.fn()
+};
+jest.mock('../../../src/lib/cloud-provider', () =>
+ jest.fn().mockImplementation(() => mockCloudProviderInstance)
+);
import cloudManagerHOC from '../../../src/lib/cloud-manager-hoc.jsx';
@@ -36,6 +42,7 @@ describe('CloudManagerHOC', () => {
vm = new VM();
vm.setCloudProvider = jest.fn();
CloudProvider.mockClear();
+ mockCloudProviderInstance.requestCloseConnection.mockClear();
});
test('when it mounts, the cloud provider is set on the vm', () => {
const Component = () => (<div />);
@@ -50,8 +57,7 @@ describe('CloudManagerHOC', () => {
);
expect(vm.setCloudProvider.mock.calls.length).toBe(1);
expect(CloudProvider).toHaveBeenCalledTimes(1);
- const cloudProviderInstance = CloudProvider.mock.instances[0];
- expect(vm.setCloudProvider).toHaveBeenCalledWith(cloudProviderInstance);
+ expect(vm.setCloudProvider).toHaveBeenCalledWith(mockCloudProviderInstance);
});
test('when cloudHost is missing, the cloud provider is not set on the vm', () => {
@@ -117,8 +123,7 @@ describe('CloudManagerHOC', () => {
});
expect(vm.setCloudProvider.mock.calls.length).toBe(1);
expect(CloudProvider).toHaveBeenCalledTimes(1);
- const cloudProviderInstance = CloudProvider.mock.instances[0];
- expect(vm.setCloudProvider).toHaveBeenCalledWith(cloudProviderInstance);
+ expect(vm.setCloudProvider).toHaveBeenCalledWith(mockCloudProviderInstance);
});
test('projectId change should not trigger cloudProvider connection unless isShowingWithId becomes true', () => {
@@ -143,8 +148,7 @@ describe('CloudManagerHOC', () => {
});
expect(vm.setCloudProvider.mock.calls.length).toBe(1);
expect(CloudProvider).toHaveBeenCalledTimes(1);
- const cloudProviderInstance = CloudProvider.mock.instances[0];
- expect(vm.setCloudProvider).toHaveBeenCalledWith(cloudProviderInstance);
+ expect(vm.setCloudProvider).toHaveBeenCalledWith(mockCloudProviderInstance);
});
test('when it unmounts, the cloud provider is set on the vm', () => {
@@ -159,9 +163,8 @@ describe('CloudManagerHOC', () => {
/>
);
- expect(CloudProvider).toHaveBeenCalledTimes(1);
- const cloudProviderInstance = CloudProvider.mock.instances[0];
- const requestCloseConnection = cloudProviderInstance.requestCloseConnection;
+ expect(CloudProvider).toHaveBeenCalled();
+ const requestCloseConnection = mockCloudProviderInstance.requestCloseConnection;
mounted.unmount();
@@ -171,4 +174,54 @@ describe('CloudManagerHOC', () => {
expect(vm.setCloudProvider).toHaveBeenCalledWith(null);
expect(requestCloseConnection).toHaveBeenCalledTimes(1);
});
+
+ test('projectId changing should trigger cloudProvider disconnection', () => {
+ const Component = () => <div />;
+ const WrappedComponent = cloudManagerHOC(Component);
+ const mounted = mount(
+ <WrappedComponent
+ cloudHost="nonEmpty"
+ store={store}
+ username="user"
+ vm={vm}
+ />
+ );
+
+ expect(CloudProvider).toHaveBeenCalled();
+ const requestCloseConnection = mockCloudProviderInstance.requestCloseConnection;
+
+ mounted.setProps({
+ projectId: 'a different id'
+ });
+
+ expect(vm.setCloudProvider.mock.calls.length).toBe(2);
+ expect(vm.setCloudProvider).toHaveBeenCalledWith(null);
+ expect(requestCloseConnection).toHaveBeenCalledTimes(1);
+
+ });
+
+ test('username changing should trigger cloudProvider disconnection', () => {
+ const Component = () => <div />;
+ const WrappedComponent = cloudManagerHOC(Component);
+ const mounted = mount(
+ <WrappedComponent
+ cloudHost="nonEmpty"
+ store={store}
+ username="user"
+ vm={vm}
+ />
+ );
+
+ expect(CloudProvider).toHaveBeenCalled();
+ const requestCloseConnection = mockCloudProviderInstance.requestCloseConnection;
+
+ mounted.setProps({
+ username: 'a different user'
+ });
+
+ expect(vm.setCloudProvider.mock.calls.length).toBe(2);
+ expect(vm.setCloudProvider).toHaveBeenCalledWith(null);
+ expect(requestCloseConnection).toHaveBeenCalledTimes(1);
+
+ });
});
--
GitLab