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

Fix broken webapp unit tests #4951

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Fix broken webapp unit tests #4951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5548c7d
add jest watch script
Dec 5, 2023
f3537e4
update snapshots
Dec 5, 2023
01e95f2
fix test
Dec 5, 2023
1ca1d44
Revert "update snapshots"
Dec 5, 2023
0c28f60
update snapshots
Dec 8, 2023
a8db585
fix broken cardFilter tests
Dec 8, 2023
f053afa
refactor
Dec 8, 2023
a7c6539
formatting
Dec 8, 2023
a25b395
fix eslint errors
Dec 8, 2023
eccd2ed
fix eslint error
Dec 8, 2023
53d9a95
fix eslint errors
Dec 9, 2023
b7b5860
remove test:watch script
Dec 10, 2023
2e30728
github ci checkout feature branch for checkout steps
Dripcoding Dec 16, 2023
b05f241
use env BRANCH_NAME
Dripcoding Dec 16, 2023
3501411
fix test
Dripcoding Dec 17, 2023
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: 4 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
View file
Open in desktop
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
View file
Open in desktop
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