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

tentative fork repro test #534

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
136 changes: 136 additions & 0 deletions example/src/tests/groupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
createGroups,
delayToPropogate,
} from './test-utils'
import {

Check warning on line 10 in example/src/tests/groupTests.ts

View workflow job for this annotation

GitHub Actions / lint

There should be at least one empty line between import groups
Client,
Conversation,
Group,
Expand All @@ -16,6 +16,7 @@
ConsentListEntry,
DecodedMessage,
} from '../../../src/index'
import { DefaultContentTypes } from 'xmtp-react-native-sdk/lib/types/DefaultContentType'

Check warning on line 19 in example/src/tests/groupTests.ts

View workflow job for this annotation

GitHub Actions / lint

`xmtp-react-native-sdk/lib/types/DefaultContentType` import should occur before import of `./test-utils`

export const groupTests: Test[] = []
let counter = 1
Expand Down Expand Up @@ -1694,3 +1695,138 @@

// return true
// })

test('groups cannot fork', async () => {
const [alix, bo, caro] = await createClients(3)
// Create group with 3 users
const { id: groupId } = await alix.conversations.newGroup([
bo.address,
caro.address,
])

const getGroupForClient = async (client: Client) => {
// Always sync the client before getting the group
await client.conversations.sync()
const group = await client.conversations.findGroup(groupId)
assert(group !== undefined, `Group not found for ${client.address}`)
return group as Group<DefaultContentTypes>
}

const syncClientAndGroup = async (client: Client) => {
const group = await getGroupForClient(client)
await group.sync()
}

const addMemberToGroup = async (fromClient: Client, addresses: string[]) => {
await syncClientAndGroup(fromClient)
const group = await getGroupForClient(fromClient)
await group.addMembers(addresses)
await delayToPropogate(500)
}

const removeMemberFromGroup = async (
fromClient: Client,
addresses: string[]
) => {
await syncClientAndGroup(fromClient)
const group = await getGroupForClient(fromClient)
await group.removeMembers(addresses)
await delayToPropogate(500)
}

// Helper to send a message from a bunch of senders and make sure it is received by all receivers
const testMessageSending = async (senderClient: Client, receiver: Client) => {
// for (const senderClient of senders) {
const messageContent = Math.random().toString(36)
await syncClientAndGroup(senderClient)
const senderGroup = await getGroupForClient(senderClient)
await senderGroup.send(messageContent)

await delayToPropogate(500)
await senderGroup.sync()

await syncClientAndGroup(receiver)

const receiverGroupToCheck = await getGroupForClient(receiver)
await receiverGroupToCheck.sync()

const messages = await receiverGroupToCheck.messages({
direction: 'DESCENDING',
})
const lastMessage = messages[0]
// console.log(lastMessage);
console.log(
`${receiverGroupToCheck.client.address} sees ${messages.length} messages in group`
)
assert(
lastMessage !== undefined &&
lastMessage.nativeContent.text === messageContent,
`${receiverGroupToCheck.client.address} should have received the message, FORK? ${lastMessage?.nativeContent.text} !== ${messageContent}`
)
// }
}

console.log('Testing that messages sent by alix are received by bo')
await testMessageSending(alix, bo)
console.log('Alix & Bo are not forked at the beginning')

// Test adding members one by one
// console.log('Testing adding members one by one...')
const newClients = await createClients(2)

// Add back several members
console.log('Adding new members to the group...')
for (const client of newClients) {
console.log(`Adding member ${client.address}...`)
await addMemberToGroup(alix, [client.address])
}
await delayToPropogate()

await alix.conversations.sync()
await syncClientAndGroup(alix)

// NB => if we don't use Promise.all but a loop, we don't get a fork
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use Promise.all we typically get a fork, the double negatives are confusing me 😆

So probably a race condition, but I guess where.

What we need to confirm from additional tests is:

  1. If we have 2 clients of the same inboxId removing members at the same time does it fork?
  2. If we have 2 clients with 2 different inboxIds does it fork?

If no to both then probably something to sure up on the SDK side

const REMOVE_MEMBERS_IN_PARALLEL = true
if (REMOVE_MEMBERS_IN_PARALLEL) {
console.log('Removing members in parallel')

await Promise.all(
newClients.map((client) => {
console.log(`Removing member ${client.address}...`)
return removeMemberFromGroup(alix, [client.address])
})
)
} else {
console.log('Removing members one by one')

for (const client of newClients) {
console.log(`Removing member ${client.address}...`)
await removeMemberFromGroup(alix, [client.address])
}
}

await delayToPropogate(1000)

// When forked, it stays forked even if we try 5 times
// but sometimes it is not forked and works 5/5 times
let forkCount = 0
const tryCount = 5
for (let i = 0; i < tryCount; i++) {
console.log(`Checking fork status ${i+1}/${tryCount}`)

Check warning on line 1815 in example/src/tests/groupTests.ts

View workflow job for this annotation

GitHub Actions / lint

Replace `+` with `·+·`
try {
await syncClientAndGroup(alix)
await syncClientAndGroup(bo)
await delayToPropogate(500)
await testMessageSending(alix, bo)
console.log('Not forked!')
} catch (e: any) {
console.log('Forked!')
console.log(e)
forkCount++
}
}

assert(forkCount === 0, `Forked ${forkCount}/${tryCount} times`)

return true
})
Loading