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

18641 More unit tests #618

Merged
merged 2 commits into from
Jan 10, 2024
Merged
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "business-create-ui",
"version": "5.6.31",
"version": "5.6.32",
"private": true,
"appName": "Create UI",
"sbcName": "SBC Common Components",
7 changes: 3 additions & 4 deletions src/components/Amalgamation/BusinessStatus.vue
Original file line number Diff line number Diff line change
@@ -58,16 +58,15 @@ export default class BusinessStatus extends Vue {
'Form Horizontal amalgamation. '

case AmlStatuses.ERROR_FOREIGN_UNLIMITED:
return 'A foreign corporation must not amalgamate with a limited company and continue as ' +
'an Unlimited Liability Company.'
return 'A foreign corporation must not amalgamate with a BC Company and continue as an ' +
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the requirement, "a BC Company of any type cannot amalgamate with a foreign company to form a BC Unlimited Liability Company," which is a join of two sets of rules (which I went over with per Yui).

'Unlimited Liability Company.'

case AmlStatuses.ERROR_FOREIGN_UNLIMITED2:
return 'A BC Company cannot amalgamate with an existing foreign corporation to form a BC ' +
'Unlimited Liability Company.'

case AmlStatuses.ERROR_FOREIGN_UNLIMITED3:
return 'A BC Company cannot amalgamate with a foreign company to form a BC Unlimited ' +
'Liability Company.'
return 'A BC Unlimited Liability Company cannot amalgamate with a foreign company.'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The code was clearly checking for a ULC so I updated this message to match, with Yui's blessing.


case AmlStatuses.ERROR_FUTURE_EFFECTIVE_FILING:
return 'This business has a future effective filing. It cannot be part of an amalgamation ' +
4 changes: 2 additions & 2 deletions src/mixin-tester.vue
Original file line number Diff line number Diff line change
@@ -4,10 +4,10 @@

<script lang="ts">
import { Component, Mixins } from 'vue-property-decorator'
import { DateMixin, FilingTemplateMixin } from '@/mixins'
import { AmalgamationMixin, DateMixin, FilingTemplateMixin } from '@/mixins'

@Component({})
export default class MixinTester extends Mixins(DateMixin, FilingTemplateMixin) {}
export default class MixinTester extends Mixins(AmalgamationMixin, DateMixin, FilingTemplateMixin) {}
</script>

<style>
18 changes: 12 additions & 6 deletions src/mixins/amalgamation-mixin.ts
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ export default class AmalgamationMixin extends Vue {

@Action(useStore) setAmalgamatingBusinesses!: (x: Array<any>) => void

/** Iterable array of rule functions, sorted by importance. */
/** Iterable array of rule functions, in order of processing. */
readonly rules = [
this.notAffiliated,
this.notHistorical,
@@ -33,6 +33,7 @@ export default class AmalgamationMixin extends Vue {
this.cccMismatch,
this.foreignUnlimited2,
this.xproUlcCcc,
this.foreignUnlimited3,
this.needBcCompany,
this.foreignHorizontal
]
@@ -91,9 +92,9 @@ export default class AmalgamationMixin extends Vue {
return null
}

/** Disallow if foreign into ULC if there is also a limited company. */
/** Disallow if foreign and there is also a BC company, into ULC. */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

'A foreign corporation must not amalgamate with a BC Company and continue as an Unlimited Liability Company.'

foreignUnlimited (business: AmalgamatingBusinessIF): AmlStatuses {
if (business.type === AmlTypes.FOREIGN && this.isTypeBcUlcCompany && this.isAnyLimited) {
if (business.type === AmlTypes.FOREIGN && this.isAnyBcCompany && this.isTypeBcUlcCompany) {
return AmlStatuses.ERROR_FOREIGN_UNLIMITED
}
return null
@@ -107,9 +108,14 @@ export default class AmalgamationMixin extends Vue {
return null
}

/** Disallow if foreign into ULC if there is also a limited company. */
/** Disallow if BC and there is also a foreign, into ULC. */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

'A BC Company cannot amalgamate with an existing foreign corporation to form a BC Unlimited Liability Company.'

foreignUnlimited2 (business: AmalgamatingBusinessIF): AmlStatuses {
if (business.type === AmlTypes.FOREIGN && this.isTypeBcUlcCompany && this.isAnyLimited) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was the same as above.

if (
business.type === AmlTypes.LEAR &&
business.legalType === CorpTypeCd.BC_COMPANY &&
this.isAnyForeign &&
this.isTypeBcUlcCompany
) {
return AmlStatuses.ERROR_FOREIGN_UNLIMITED2
}
return null
@@ -138,7 +144,7 @@ export default class AmalgamationMixin extends Vue {
return null
}

/** Disallow if there are no BC Companies. */
/** Disallow if there are no BC companies. */
needBcCompany (): AmlStatuses {
if (!this.isAnyBcCompany) {
return AmlStatuses.ERROR_NEED_BC_COMPANY
4 changes: 2 additions & 2 deletions tests/unit/BusinessStatus.spec.ts
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ describe('Business Status', () => {
{
label: 'Error Foreign Unlimited',
status: AmlStatuses.ERROR_FOREIGN_UNLIMITED,
tooltip: 'A foreign corporation must not amalgamate with a limited company and continue as'
tooltip: 'A foreign corporation must not amalgamate with a BC Company and continue as an'
},
{
label: 'Error Foreign Unlimited 2',
@@ -37,7 +37,7 @@ describe('Business Status', () => {
{
label: 'Error Foreign Unlimited 3',
status: AmlStatuses.ERROR_FOREIGN_UNLIMITED3,
tooltip: 'A BC Company cannot amalgamate with a foreign company to form a BC Unlimited'
tooltip: 'A BC Unlimited Liability Company cannot amalgamate with a foreign company.'
},
{
label: 'Error Future Effective Filing',
81 changes: 79 additions & 2 deletions tests/unit/BusinessTable.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { AmlRoles, AmlTypes } from '@/enums'
import { AmlRoles, AmlStatuses, AmlTypes } from '@/enums'
import { wrapperFactory } from '../vitest-wrapper-factory'
import BusinessTable from '@/components/Amalgamation/BusinessTable.vue'
import { CorpTypeCd } from '@bcrs-shared-components/corp-type-module'
import AmalgamationMixin from '@/mixins/amalgamation-mixin'

describe('Business Table', () => {
describe('Business Table - display', () => {
it('displays correctly when there are no amalgamating businesses', () => {
const wrapper = wrapperFactory(
BusinessTable,
@@ -167,3 +168,79 @@ describe('Business Table', () => {
})
}
})

// *** FUTURE: get this working
// ATM, local rules are mocked (eg, wrapper.vm.notAffiliated()), but not the actual rules in the mixin.
// It's probably this: https://vitest.dev/guide/mocking.html#mocking-pitfalls.
// Maybe try vi.mock() to mock the imported mixin?
describe.skip('Business Table - rule evaluation', () => {
let wrapper: any
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Jan 10, 2024

Choose a reason for hiding this comment

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

I spend quite some time trying to get this working. The idea was to mock (override) all the rules, and then clear them one by one, to verify that the map/reduce function was working correctly.

However, while I could mock the rules locally in wrapper.vm (the mounted component under test), this was not overriding the rules in the mixin :(

Anyway, someone in the future can look at this if they have time and feel like it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wonder if we should add this as a checkbox somewhere so that we don't forget about it Sev. Anyway, OK with me 👍

Copy link
Collaborator Author

@severinbeauvais severinbeauvais Jan 10, 2024

Choose a reason for hiding this comment

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

Nah. There are lots of other unit tests we could write that we don't keep track of. Mostly I didn't want to throw away this code which partly works :)


const rules = [
{ methodName: 'notAffiliated', error: AmlStatuses.ERROR_NOT_AFFILIATED },
{ methodName: 'notHistorical', error: AmlStatuses.ERROR_HISTORICAL },
{ methodName: 'notInGoodStanding', error: AmlStatuses.ERROR_NOT_IN_GOOD_STANDING },
{ methodName: 'limitedRestoration', error: AmlStatuses.ERROR_LIMITED_RESTORATION },
{ methodName: 'futureEffectiveFiling', error: AmlStatuses.ERROR_FUTURE_EFFECTIVE_FILING },
{ methodName: 'foreign', error: AmlStatuses.ERROR_FOREIGN },
{ methodName: 'foreignUnlimited', error: AmlStatuses.ERROR_FOREIGN_UNLIMITED },
{ methodName: 'cccMismatch', error: AmlStatuses.ERROR_CCC_MISMATCH },
{ methodName: 'foreignUnlimited2', error: AmlStatuses.ERROR_FOREIGN_UNLIMITED2 },
{ methodName: 'xproUlcCcc', error: AmlStatuses.ERROR_XPRO_ULC_CCC },
{ methodName: 'needBcCompany', error: AmlStatuses.ERROR_NEED_BC_COMPANY },
{ methodName: 'foreignHorizontal', error: AmlStatuses.ERROR_FOREIGN_HORIZONTAL }
]

beforeAll(() => {
wrapper = wrapperFactory(
BusinessTable,
null,
{
amalgamation: {
amalgamatingBusinesses: [{ /* dummy business */ }]
},
tombstone: {
keycloakRoles: ['staff']
}
}
)

// mock all rules to return their error
for (let i = 0; i < rules.length; i++) {
vi.spyOn(wrapper.vm, rules[i].methodName).mockReturnValue(rules[i].error)
}
// *** these work
console.log('*** value1 = ', wrapper.vm.notAffiliated())
console.log('*** value2 = ', wrapper.vm.notHistorical())
console.log('*** value3 = ', wrapper.vm.notInGoodStanding())

// *** this doesn't work ("is not a function")
console.log('*** value4 = ', (AmalgamationMixin as any).notAffiliated())
})

it('has the expected number of rules', () => {
expect(wrapper.vm.rules.length).toBe(rules.length)
})

// check each rule sequentially
for (let i = 0; i < rules.length; i++) {
it.skip(`fails rule "${rules[i].methodName}"`, () => {
// first, verify that current rule fails
expect(wrapper.vm.businesses[0].status).toBe(rules[i].error)

// now override rule to return null
vi.spyOn(wrapper.vm, rules[i].methodName).mockReturnValue(null)

// if this is the last rule...
if (i === (rules.length - 1)) {
// verify that no rules have failed
expect(wrapper.vm.businesses[0].status).toBe(AmlStatuses.OK)
} else {
// verify that another rule has failed
expect(wrapper.vm.businesses[0].status).toBe(rules[i + 1].error)
}

wrapper.destroy()
})
}
})
Loading