Skip to content

Commit

Permalink
#554 Addressed unit test errors and missing coverage. Cleaned up unus…
Browse files Browse the repository at this point in the history 
…ed files and/or blocks of code. Cleaned up/clarified files per findings from tests.
  • Loading branch information
@FrenjaminBanklin
FrenjaminBanklin committed Nov 21, 2023
1 parent 1f35d07 commit a914b38
Show file tree
Hide file tree
Showing 43 changed files with 3,757 additions and 1,426 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"test:ci": "TZ='America/New_York' CI=true jest --ci --useStderr --coverage --coverageReporters text-summary cobertura",
"test:ci:each": "lerna run test:ci",
"test:dev": "TZ='America/New_York' jest --verbose --watchAll --coverage --coverageReporters lcov",
"test:view-lcov": "open ./coverage/lcov-report/index.html",
"postinstall": "husky install"
},
"devDependencies": {
Expand Down
143 changes: 143 additions & 0 deletions packages/app/obojobo-document-engine/__tests__/Viewer/components/nav.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ describe('Nav', () => {

beforeEach(() => {
jest.clearAllMocks()
mockDispatcherTrigger.mockReset()
})

test('renders opened', () => {
Expand Down Expand Up @@ -195,6 +196,148 @@ describe('Nav', () => {
expect(tree).toMatchSnapshot()
})

test('does not try to substitute labels if no brackets are found', () => {
// this shouldn't run in this test, but if it does the label will be changed
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = 'not-label5'
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: 'label5',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
expect(liElements[1].children[0].children[0]).toBe('label5')
expect(mockDispatcherTrigger).not.toHaveBeenCalled()
})

test('does not try to substitute labels if the variable text is not correctly formatted', () => {
// this shouldn't run in this test, but if it does the label will be changed
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = 'not-label5'
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: '{{label5',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
expect(liElements[1].children[0].children[0]).toBe('{{label5')
expect(mockDispatcherTrigger).not.toHaveBeenCalled()
})

test('renders substituted variables in labels - substitute exists', () => {
// this should run and as a result modify the label text
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = 'not-label5'
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: '{{$var}}',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
expect(liElements[1].children[0].children[0]).toBe('not-label5')
expect(mockDispatcherTrigger).toHaveBeenCalledTimes(1)
// so this second argument is weird - it isn't the thing that was originally sent to Dispatch.trigger,
// which was { text: '', isNav: true }, but because the callback triggered changed a property of the
// object it was passed, then it was technically also changing the object it was passed, which means
// we have to consider the state that object would have been in after the callback ran
expect(mockDispatcherTrigger).toHaveBeenCalledWith(
'getTextForVariable',
{ text: 'not-label5', isNav: true },
'$var',
{}
)
})

test('renders substituted variables in labels - no substitute exists', () => {
// this should run but as a result not modify the label text
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = null
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: '{{$var}}',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
// event text was unchanged so original label text should be used
expect(liElements[1].children[0].children[0]).toBe('{{$var}}')
expect(mockDispatcherTrigger).toHaveBeenCalledTimes(1)
// see previous test note re: the second arg
expect(mockDispatcherTrigger).toHaveBeenCalledWith(
'getTextForVariable',
{ text: null, isNav: true },
'$var',
{}
)
})

test('renders blank title', () => {
NavUtil.getOrderedList.mockReturnValueOnce([{ id: 4, type: 'heading', label: '' }])
const props = {
Expand Down
157 changes: 156 additions & 1 deletion packages/app/obojobo-document-engine/__tests__/Viewer/components/viewer-app.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import { mount } from 'enzyme'
import testObject from 'obojobo-document-engine/test-object.json'
import mockConsole from 'jest-mock-console'
import injectKatexIfNeeded from 'obojobo-document-engine/src/scripts/common/util/inject-katex-if-needed'
import VariableStore from 'obojobo-document-engine/src/scripts/viewer/stores/variable-store'
import VariableUtil from 'obojobo-document-engine/src/scripts/viewer/util/variable-util'

jest.mock('obojobo-document-engine/src/scripts/viewer/util/viewer-api')
jest.mock('obojobo-document-engine/src/scripts/common/util/inject-katex-if-needed')
Expand All @@ -40,6 +42,8 @@ jest.mock('obojobo-document-engine/src/scripts/viewer/components/nav')
jest.mock('obojobo-document-engine/src/scripts/common/page/dom-util')
jest.mock('obojobo-document-engine/src/scripts/common/util/insert-dom-tag')
jest.mock('obojobo-document-engine/src/scripts/common/components/modal-container')
jest.mock('obojobo-document-engine/src/scripts/viewer/stores/variable-store')
jest.mock('obojobo-document-engine/src/scripts/viewer/util/variable-util')

describe('ViewerApp', () => {
let restoreConsole
Expand Down Expand Up @@ -1025,7 +1029,7 @@ describe('ViewerApp', () => {
})
})

test('sendCloseEvent calls navigator.sendBeacon', done => {
test('sendClose`Event calls navigator.sendBeacon', done => {
global.navigator.sendBeacon = jest.fn()

expect.assertions(1)
Expand Down Expand Up @@ -1841,4 +1845,155 @@ describe('ViewerApp', () => {
spy1.mockRestore()
spy2.mockRestore()
})

test('component passes variables to VariableStore correctly', done => {
mocksForMount()

const mockVariables = {
'nodeid1:variablename1': 'var1',
'nodeid1:variablename2': 'var2'
}

// reset the mocked function to test variables
ViewerAPI.requestStart = jest.fn().mockResolvedValueOnce({
status: 'ok',
value: {
visitId: 123,
lti: {
lisOutcomeServiceUrl: 'http://lis-outcome-service-url.test/example.php'
},
isPreviewing: true,
extensions: {
':ObojoboDraft.Sections.Assessment:attemptHistory': []
},
variables: mockVariables
}
})
const component = mount(<ViewerApp />)

setTimeout(() => {
expect(VariableStore.init).toHaveBeenCalledWith(mockVariables)
component.update()
done()
})
})

test('component calls expected VariableUtil functions, standard variable name', done => {
mocksForMount()

NavUtil.getContext.mockReturnValueOnce('test-context')

const mockVariables = {
'nodeid1:variablename1': 'var1',
'nodeid1:variablename2': 'var2'
}

// reset the mocked function to test variables
ViewerAPI.requestStart = jest.fn().mockResolvedValueOnce({
status: 'ok',
value: {
visitId: 123,
lti: {
lisOutcomeServiceUrl: 'http://lis-outcome-service-url.test/example.php'
},
isPreviewing: true,
extensions: {
':ObojoboDraft.Sections.Assessment:attemptHistory': []
},
variables: mockVariables
}
})

const mockVariableState = { mockVariableStateKey: 'mockVariableStateVal' }

VariableStore.getState.mockReturnValueOnce(mockVariableState)

const component = mount(<ViewerApp />)

setTimeout(() => {
component.update()

VariableUtil.findValueWithModel.mockReturnValueOnce('mock-var-value')

// ordinarily this is an oboModel instance, but it doesn't matter for tests
const mockTextModel = { key: 'val' }
const mockEvent = { text: '' }

component.instance().getTextForVariable(mockEvent, '$variablename1', mockTextModel)
expect(VariableUtil.findValueWithModel).toHaveBeenCalledWith(
'test-context',
mockVariableState,
mockTextModel,
'variablename1'
)
expect(VariableUtil.getValue).not.toHaveBeenCalled()

expect(mockEvent.text).toEqual('mock-var-value')

done()
})
})

test('component calls expected VariableUtil functions, owner:variable name', done => {
mocksForMount()

NavUtil.getContext.mockReturnValueOnce('test-context')

const mockVariables = {
'nodeid1:variablename1': 'var1',
'nodeid1:variablename2': 'var2'
}

// reset the mocked function to test variables
ViewerAPI.requestStart = jest.fn().mockResolvedValueOnce({
status: 'ok',
value: {
visitId: 123,
lti: {
lisOutcomeServiceUrl: 'http://lis-outcome-service-url.test/example.php'
},
isPreviewing: true,
extensions: {
':ObojoboDraft.Sections.Assessment:attemptHistory': []
},
variables: mockVariables
}
})

const mockVariableState = { mockVariableStateKey: 'mockVariableStateVal' }

VariableStore.getState.mockReturnValueOnce(mockVariableState)

const component = mount(<ViewerApp />)

setTimeout(() => {
component.update()

VariableUtil.getValue.mockReturnValueOnce('mock-var-value')

// ordinarily this is an oboModel instance, but it doesn't matter for tests
const mockTextModel = { key: 'val' }
const mockEvent = { text: '' }

component.instance().getTextForVariable(mockEvent, '$nodeid1:variablename1', mockTextModel)
expect(VariableUtil.getValue).toHaveBeenCalledWith(
'test-context',
mockVariableState,
'nodeid1',
'variablename1'
)
expect(VariableUtil.findValueWithModel).not.toHaveBeenCalled()

expect(mockEvent.text).toEqual('mock-var-value')

// bonus test to make sure event text is not overwritten if no value is found
VariableUtil.getValue.mockReturnValueOnce(null)
mockEvent.text = ''
expect(mockEvent.text).toEqual('')
component.instance().getTextForVariable(mockEvent, '$nodeid1:variablename1', mockTextModel)
expect(mockEvent.text).toEqual('')

done()
})
})
})
Loading

0 comments on commit a914b38

Please sign in to comment.