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

Fix broken webapp unit tests #4951

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jobs:
uses: actions/checkout@v3
with:
path: "focalboard"
ref: ${{ env.BRANCH_NAME }}
Copy link
Author

Choose a reason for hiding this comment

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

this ensures that the feature branch is checked out when the workflows run

- name: Set up Go
uses: actions/setup-go@v3
with:
Expand All @@ -45,6 +46,7 @@ jobs:
uses: actions/checkout@v3
with:
path: "focalboard"
ref: ${{ env.BRANCH_NAME }}
- name: npm ci
run: |
cd focalboard/webapp && npm ci && cd -
Expand Down Expand Up @@ -88,6 +90,7 @@ jobs:
uses: actions/checkout@v3
with:
path: "focalboard"
ref: ${{ env.BRANCH_NAME }}

- name: Set up Go
uses: actions/setup-go@v3
Expand All @@ -110,6 +113,7 @@ jobs:
uses: actions/checkout@v3
with:
path: "focalboard"
ref: ${{ env.BRANCH_NAME }}

- name: Set up Go
uses: actions/setup-go@v3
Expand Down
129 changes: 74 additions & 55 deletions webapp/src/cardFilter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,59 +10,76 @@ import {TestBlockFactory} from './test/testBlockFactory'
import {Utils} from './utils'

import {IPropertyTemplate} from './blocks/board'
import {blockTypes} from './blocks/block'

jest.mock('./utils')
const mockedUtils = mocked(Utils, true)

const dayMillis = 24 * 60 * 60 * 1000

describe('src/cardFilter', () => {
const board = TestBlockFactory.createBoard()
board.id = '1'
const board = TestBlockFactory.createBoard()
board.id = '1'

const block = {
id: 'id',
boardId: 'boardId',
parentId: 'parnetId',
createdBy: 'createdBy',
modifiedBy: 'modifiedBy',
schema: 1,
type: blockTypes[13],
title: 'title',
fields: {
properties: {
propertyId: 'Status',
},
},
createAt: new Date('December 7, 2023').getTime(),
updateAt: new Date('December 7, 2023').getTime(),
deleteAt: new Date('December 7, 2023').getTime(),
}
const card = TestBlockFactory.createCardWithBlock(board, block)

const card1 = TestBlockFactory.createCard(board)
Copy link
Author

Choose a reason for hiding this comment

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

This function relies on Date.now() to populate the createAt and updatedAt fields when a block is not provided. I created a new function createCardWithBlock and passed it a block stub to make it clearer how card fields are populated for this test suite.

Copy link
Contributor

@Rajat-Dabade Rajat-Dabade May 29, 2024

Choose a reason for hiding this comment

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

This makes sense, as at some point in time we might test cases for checking createAt, updateAt and other stuff.
Even though we can directly assign it to the card object, the function makes more sense to avoid duplicity. It is also better to have a concrete time rather than depending on the Date.now because some test cases can fail based on the time and regions.

card1.id = '1'
card1.title = 'card1'
card1.fields.properties.propertyId = 'Status'
describe('src/cardFilter', () => {
const filterClause = createFilterClause({propertyId: 'propertyId', condition: 'isNotEmpty', values: ['Status']})

describe('verify isClauseMet method', () => {
test('should be true with isNotEmpty clause', () => {
const filterClauseIsNotEmpty = createFilterClause({propertyId: 'propertyId', condition: 'isNotEmpty', values: ['Status']})
const result = CardFilter.isClauseMet(filterClauseIsNotEmpty, [], card1)
const result = CardFilter.isClauseMet(filterClauseIsNotEmpty, [], card)
expect(result).toBeTruthy()
})
test('should be false with isEmpty clause', () => {
const filterClauseIsEmpty = createFilterClause({propertyId: 'propertyId', condition: 'isEmpty', values: ['Status']})
const result = CardFilter.isClauseMet(filterClauseIsEmpty, [], card1)
const result = CardFilter.isClauseMet(filterClauseIsEmpty, [], card)
expect(result).toBeFalsy()
})
test('should be true with includes clause', () => {
const filterClauseIncludes = createFilterClause({propertyId: 'propertyId', condition: 'includes', values: ['Status']})
const result = CardFilter.isClauseMet(filterClauseIncludes, [], card1)
const result = CardFilter.isClauseMet(filterClauseIncludes, [], card)
expect(result).toBeTruthy()
})
test('should be true with includes and no values clauses', () => {
const filterClauseIncludes = createFilterClause({propertyId: 'propertyId', condition: 'includes', values: []})
const result = CardFilter.isClauseMet(filterClauseIncludes, [], card1)
const result = CardFilter.isClauseMet(filterClauseIncludes, [], card)
expect(result).toBeTruthy()
})
test('should be false with notIncludes clause', () => {
const filterClauseNotIncludes = createFilterClause({propertyId: 'propertyId', condition: 'notIncludes', values: ['Status']})
const result = CardFilter.isClauseMet(filterClauseNotIncludes, [], card1)
const result = CardFilter.isClauseMet(filterClauseNotIncludes, [], card)
expect(result).toBeFalsy()
})
test('should be true with notIncludes and no values clauses', () => {
const filterClauseNotIncludes = createFilterClause({propertyId: 'propertyId', condition: 'notIncludes', values: []})
const result = CardFilter.isClauseMet(filterClauseNotIncludes, [], card1)
const result = CardFilter.isClauseMet(filterClauseNotIncludes, [], card)
expect(result).toBeTruthy()
})
})

describe('verify isClauseMet method - person property', () => {
const personCard = TestBlockFactory.createCard(board)
personCard.id = '1'
personCard.title = 'card1'
personCard.title = 'card'
personCard.fields.properties.personPropertyID = 'personid1'

const template: IPropertyTemplate = {
Expand Down Expand Up @@ -107,7 +124,7 @@ describe('src/cardFilter', () => {
describe('verify isClauseMet method - multi-person property', () => {
const personCard = TestBlockFactory.createCard(board)
personCard.id = '1'
personCard.title = 'card1'
personCard.title = 'card'
personCard.fields.properties.personPropertyID = ['personid1', 'personid2']

const template: IPropertyTemplate = {
Expand Down Expand Up @@ -167,7 +184,7 @@ describe('src/cardFilter', () => {
describe('verify isClauseMet method - (createdBy) person property', () => {
const personCard = TestBlockFactory.createCard(board)
personCard.id = '1'
personCard.title = 'card1'
personCard.title = 'card'
personCard.createdBy = 'personid1'

const template: IPropertyTemplate = {
Expand Down Expand Up @@ -206,7 +223,7 @@ describe('src/cardFilter', () => {

const dateCard = TestBlockFactory.createCard(board)
dateCard.id = '1'
dateCard.title = 'card1'
dateCard.title = 'card'
dateCard.fields.properties.datePropertyID = '{ "from": ' + propertyDate.toString() + ' }'

const checkDayBefore = propertyDate - dayMillis
Expand Down Expand Up @@ -265,7 +282,7 @@ describe('src/cardFilter', () => {
const toDate = fromDate + (2 * dayMillis)
const dateCard = TestBlockFactory.createCard(board)
dateCard.id = '1'
dateCard.title = 'card1'
dateCard.title = 'card'
dateCard.fields.properties.datePropertyID = '{ "from": ' + fromDate.toString() + ', "to": ' + toDate.toString() + ' }'

const beforeRange = fromDate - dayMillis
Expand Down Expand Up @@ -335,11 +352,6 @@ describe('src/cardFilter', () => {
})

describe('verify isClauseMet method - (createdTime) date property', () => {
const createDate = new Date(card1.createAt)
const checkDate = Date.UTC(createDate.getFullYear(), createDate.getMonth(), createDate.getDate(), 12)
const checkDayBefore = checkDate - dayMillis
const checkDayAfter = checkDate + dayMillis

const template: IPropertyTemplate = {
id: 'datePropertyID',
name: 'myDate',
Expand All @@ -349,44 +361,51 @@ describe('src/cardFilter', () => {

test('should be true with isSet clause', () => {
const filterClauseIsSet = createFilterClause({propertyId: 'datePropertyID', condition: 'isSet', values: []})
const result = CardFilter.isClauseMet(filterClauseIsSet, [template], card1)
const result = CardFilter.isClauseMet(filterClauseIsSet, [template], card)
expect(result).toBeTruthy()
})
test('should be false with notSet clause', () => {
const filterClauseIsNotSet = createFilterClause({propertyId: 'datePropertyID', condition: 'isNotSet', values: []})
const result = CardFilter.isClauseMet(filterClauseIsNotSet, [template], card1)
const result = CardFilter.isClauseMet(filterClauseIsNotSet, [template], card)
expect(result).toBeFalsy()
})
test('verify isBefore clause', () => {
const filterClauseIsBefore = createFilterClause({propertyId: 'datePropertyID', condition: 'isBefore', values: [checkDayAfter.toString()]})
const result = CardFilter.isClauseMet(filterClauseIsBefore, [template], card1)
const filterDateSetInFuture = new Date('December 31, 2023').getTime()
const filterClauseIsBefore = createFilterClause({propertyId: 'datePropertyID', condition: 'isBefore', values: [filterDateSetInFuture.toString()]})
const result = CardFilter.isClauseMet(filterClauseIsBefore, [template], card)
expect(result).toBeTruthy()

const filterClauseIsNotBefore = createFilterClause({propertyId: 'datePropertyID', condition: 'isBefore', values: [checkDate.toString()]})
const result2 = CardFilter.isClauseMet(filterClauseIsNotBefore, [template], card1)
const filterDateSetInPast = new Date('December 6, 2022').getTime()
const filterClauseIsNotBefore = createFilterClause({propertyId: 'datePropertyID', condition: 'isBefore', values: [filterDateSetInPast.toString()]})
const result2 = CardFilter.isClauseMet(filterClauseIsNotBefore, [template], card)
expect(result2).toBeFalsy()
})
test('verify isAfter clauses', () => {
const filterClauseisAfter = createFilterClause({propertyId: 'datePropertyID', condition: 'isAfter', values: [checkDayBefore.toString()]})
const result = CardFilter.isClauseMet(filterClauseisAfter, [template], card1)
const filterDateSetInPast = new Date('December 6, 2022').getTime()
const filterClauseisAfter = createFilterClause({propertyId: 'datePropertyID', condition: 'isAfter', values: [filterDateSetInPast.toString()]})
const result = CardFilter.isClauseMet(filterClauseisAfter, [template], card)
expect(result).toBeTruthy()

const filterClauseisNotAfter = createFilterClause({propertyId: 'datePropertyID', condition: 'isAfter', values: [checkDate.toString()]})
const result2 = CardFilter.isClauseMet(filterClauseisNotAfter, [template], card1)
const filterDateSetInFuture = new Date('December 31, 2023').getTime()
const filterClauseisNotAfter = createFilterClause({propertyId: 'datePropertyID', condition: 'isAfter', values: [filterDateSetInFuture.toString()]})
const result2 = CardFilter.isClauseMet(filterClauseisNotAfter, [template], card)
expect(result2).toBeFalsy()
})
test('verify is clause', () => {
// Is should find on that date regardless of time.
const filterClauseIs = createFilterClause({propertyId: 'datePropertyID', condition: 'is', values: [checkDate.toString()]})
const result = CardFilter.isClauseMet(filterClauseIs, [template], card1)
const filterDateSetToCurrent = new Date('December 7, 2023').getTime()
const filterClauseIs = createFilterClause({propertyId: 'datePropertyID', condition: 'is', values: [filterDateSetToCurrent.toString()]})
const result = CardFilter.isClauseMet(filterClauseIs, [template], card)
expect(result).toBeTruthy()

const filterClauseIsNot = createFilterClause({propertyId: 'datePropertyID', condition: 'is', values: [checkDayBefore.toString()]})
const result2 = CardFilter.isClauseMet(filterClauseIsNot, [template], card1)
const filterDateSetInPast = new Date('December 6, 2023')
const filterClauseIsNot = createFilterClause({propertyId: 'datePropertyID', condition: 'is', values: [filterDateSetInPast.toString()]})
const result2 = CardFilter.isClauseMet(filterClauseIsNot, [template], card)
expect(result2).toBeFalsy()

const filterClauseIsNot2 = createFilterClause({propertyId: 'datePropertyID', condition: 'is', values: [checkDayAfter.toString()]})
const result3 = CardFilter.isClauseMet(filterClauseIsNot2, [template], card1)
const filterDateSetInFuture = new Date('December 21, 2023')
const filterClauseIsNot2 = createFilterClause({propertyId: 'datePropertyID', condition: 'is', values: [filterDateSetInFuture.toString()]})
const result3 = CardFilter.isClauseMet(filterClauseIsNot2, [template], card)
expect(result3).toBeFalsy()
})
})
Expand All @@ -397,7 +416,7 @@ describe('src/cardFilter', () => {
operation: 'and',
filters: [],
})
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeTruthy()
})
test('should return true with or operation and 2 filterCause, one is false ', () => {
Expand All @@ -409,7 +428,7 @@ describe('src/cardFilter', () => {
filterClause,
],
})
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeTruthy()
})
test('should return true with or operation and 2 filterCause, 1 filtergroup in filtergroup, one filterClause is false ', () => {
Expand All @@ -426,7 +445,7 @@ describe('src/cardFilter', () => {
filters: [],
})
filterGroup.filters.push(filterGroupInFilterGroup)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeTruthy()
})
test('should return false with or operation and two filterCause, two are false ', () => {
Expand All @@ -439,7 +458,7 @@ describe('src/cardFilter', () => {
filterClauseEmpty,
],
})
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeFalsy()
})
test('should return false with and operation and 2 filterCause, one is false ', () => {
Expand All @@ -451,7 +470,7 @@ describe('src/cardFilter', () => {
filterClause,
],
})
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeFalsy()
})
test('should return true with and operation and 2 filterCause, two are true ', () => {
Expand All @@ -463,7 +482,7 @@ describe('src/cardFilter', () => {
filterClause,
],
})
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeTruthy()
})
test('should return true with or operation and 2 filterCause, 1 filtergroup in filtergroup, one filterClause is false ', () => {
Expand All @@ -480,7 +499,7 @@ describe('src/cardFilter', () => {
filters: [],
})
filterGroup.filters.push(filterGroupInFilterGroup)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card1)
const result = CardFilter.isFilterGroupMet(filterGroup, [], card)
expect(result).toBeFalsy()
})
})
Expand Down Expand Up @@ -685,7 +704,7 @@ describe('src/cardFilter', () => {
})
})
describe('verify applyFilterGroup method', () => {
test('should return array with card1', () => {
test('should return array with card', () => {
const filterClauseNotIncludes = createFilterClause({propertyId: 'propertyId', condition: 'notIncludes', values: ['Status']})
const filterGroup = createFilterGroup({
operation: 'or',
Expand All @@ -694,34 +713,34 @@ describe('src/cardFilter', () => {
filterClause,
],
})
const result = CardFilter.applyFilterGroup(filterGroup, [], [card1])
const result = CardFilter.applyFilterGroup(filterGroup, [], [card])
expect(result).toBeDefined()
expect(result[0]).toEqual(card1)
expect(result[0]).toEqual(card)
})
})
describe('verfiy applyFilterGroup method for case-sensitive search', () => {
test('should return array with card1 when search by test as Card1', () => {
const filterClauseNotContains = createFilterClause({propertyId: 'title', condition: 'contains', values: ['Card1']})
test('should return array with card when search by test as card', () => {
const filterClauseNotContains = createFilterClause({propertyId: 'title', condition: 'contains', values: ['title']})
const filterGroup = createFilterGroup({
operation: 'and',
filters: [
filterClauseNotContains,
],
})
const result = CardFilter.applyFilterGroup(filterGroup, [], [card1])
const result = CardFilter.applyFilterGroup(filterGroup, [], [card])
expect(result.length).toEqual(1)
})
})
describe('verify applyFilter for title', () => {
test('should not return array with card1', () => {
const filterClauseNotContains = createFilterClause({propertyId: 'title', condition: 'notContains', values: ['card1']})
test('should not return array with card', () => {
const filterClauseNotContains = createFilterClause({propertyId: 'title', condition: 'notContains', values: ['title']})
const filterGroup = createFilterGroup({
operation: 'and',
filters: [
filterClauseNotContains,
],
})
const result = CardFilter.applyFilterGroup(filterGroup, [], [card1])
const result = CardFilter.applyFilterGroup(filterGroup, [], [card])
expect(result.length).toEqual(0)
})
})
Expand Down
9 changes: 9 additions & 0 deletions webapp/src/test/testBlockFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,15 @@ class TestBlockFactory {
return card
}

static createCardWithBlock(board?: Board, block?: Block): Card {
const card = createCard(block)
card.boardId = board ? board.id : 'board'
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see the same code is repeated here as well,

https://github.com/Dripcoding/focalboard/blob/3501411a61f498cb715da5a4e87f696d630eb045/webapp/src/test/testBlockFactory.ts#L117

better to reuse it rather than duplicating it.

card.title = 'title'
card.fields.icon = 'i'
card.fields.properties.property1 = 'value1'
return card
}

private static addToCard<BlockType extends Block>(block: BlockType, card: Card, isContent = true): BlockType {
block.parentId = card.id
block.boardId = card.boardId
Expand Down
3 changes: 2 additions & 1 deletion webapp/src/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ describe('utils', () => {
it('should show month, day and time for current year', () => {
const currentYear = new Date().getFullYear()
const date = new Date(currentYear, 6, 9, 15, 20)
expect(Utils.displayDateTime(date, intl)).toBe('July 09, 3:20 PM')
const actual = Utils.displayDateTime(date, intl)
expect(actual).toBe('July 09, 3:20 PM')
Copy link
Contributor

Choose a reason for hiding this comment

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

This is failing

Expected is: July 09 at 3:20 PM, Please check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, we are bumping up the node version to v20.11.1 so please make sure you use it for testing.

})

it('should show month, day, year and time for previous year', () => {
Expand Down
Loading