Skip to content

Commit

Permalink
Fix heisenbug of group-chat (#2231)
Browse files Browse the repository at this point in the history
* chore: added a small update and Cypress try

* chore: added comment and Cypress retry

* chore: check if group-chat passes three times in a row

* chore: group-chat passes 4 times in a row

* chore: 5 times pass in a row means it fixes the heisenbug? maybe it's because assert inside then clause?

* fix: added cy.wait and Cypress try

* chore: added comment and Cypress retry

* fix: removing cy.wait

* chore: updated comment and check Cypress passed 2 times in a row

* chore: really passes 3 times in a row?
  • Loading branch information
Silver-IT committed Jul 23, 2024
1 parent 4ed1baa commit 06aabd6
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
8 changes: 8 additions & 0 deletions frontend/controller/actions/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,14 @@ export default (sbp('sbp/selectors/register', {
const response = await sendMessage({ ...params, data: { ...data, passPayload } })

if (proposalToSend) {
// NOTE: sometimes 'notifyProposalStateInGeneralChatRoom' function could be called
// after the 'proposalVote' event is finished its processing (published, received, called process/sideEffect)
// it's not a big problem unless the proposal to remove someone from the group is accepted by the current vote.
// it's when the two messages will be created in general chatroom; one is to tells the proposal is approved
// and the other is to tell the member is left general chatroom.
// the order of two functions that will be called means the order of the two messages in general chatroom.
// so the order of messages isn't always same and that could cause the heisenbug below
// https://github.com/okTurtles/group-income/tree/2226-heisenbug-in-group-chatspecjs-more-persistent-one
await sbp('gi.actions/group/notifyProposalStateInGeneralChatRoom', {
groupID: contractID,
proposal: proposalToSend
Expand Down
34 changes: 21 additions & 13 deletions test/cypress/integration/group-chat.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,30 @@ describe('Group Chat Basic Features (Create & Join & Leave & Close)', () => {
// considering it sync the chatroom contract from the beginning
} else {
const message = selfLeave ? `Left ${channelName}` : `Kicked a member from ${channelName}: ${leaver}`
const messageSelectors = {
last: 'div.c-message:last-child',
secondLast: 'div.c-message:nth-last-child(2)'
}
const assertKickerAndMessageContent = (msgSelector) => {
cy.get(`${msgSelector} .c-who > span:first-child`).should('contain', kicker)
cy.get(`${msgSelector} .c-notification`).should('contain', message)
}

let isLastElement = true
if (byProposal) {
// NOTE: when the member is kicked from the from by proposal
// two messages will be created in general chatroom; INTERACTIVE, and NOTIFICATION
// INTERACTIVE message should be created before the NOTIFICATION message
// but sometimes (only in Cypress) NOTIFICATION message could be created earlier
// this block is to handle that heisenbug
cy.wait(1000) // eslint-disable-line cypress/no-unnecessary-waiting
cy.get('div.c-message:last-child').invoke('attr', 'class').then(classNames => {
isLastElement = classNames.includes('is-type-notification')
// but sometimes (mostly in Cypress) NOTIFICATION message could be created earlier
// and the order of two messages could be changed and it could cause the heisenbug.
// the below block is to handle that heisenbug
cy.get(messageSelectors.last).invoke('attr', 'class').then(classNames => {
const isLastMsgTypeNotification = classNames.includes('is-type-notification')
assertKickerAndMessageContent(
isLastMsgTypeNotification ? messageSelectors.last : messageSelectors.secondLast
)
})
}

if (isLastElement) {
cy.get('div.c-message:last-child .c-who > span:first-child').should('contain', kicker)
cy.get('div.c-message:last-child .c-notification').should('contain', message)
} else {
cy.get('div.c-message:nth-last-child(2) .c-who > span:first-child').should('contain', kicker)
cy.get('div.c-message:nth-last-child(2) .c-notification').should('contain', message)
assertKickerAndMessageContent(messageSelectors.last)
}
}
}
Expand Down Expand Up @@ -411,6 +415,10 @@ describe('Group Chat Basic Features (Create & Join & Leave & Close)', () => {

cy.getByDT('groupMembers').find('ul>li').should('have.length', 2) // user1 & user2

// NOTE: this check is to wait until 2 INTERACTIVE mesages are created
// one for creating proposal and another is for proposal approval
cy.getByDT('groupChatLink').get('.c-badge.is-compact[aria-label="2 new notifications"]').contains('2')

cy.giRedirectToGroupChat()

cy.giSwitchChannel(CHATROOM_GENERAL_NAME)
Expand Down

0 comments on commit 06aabd6

Please sign in to comment.