From 08e8a48eedc8fe684b4f745d92d8e06e182b2b99 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 9 Oct 2024 13:28:12 +1100 Subject: [PATCH 1/8] Optimised the libSession size limit tests, tweaked another couple failing tests --- .../LibSession/LibSessionUtilSpec.swift | 147 +++++++++++------- 1 file changed, 89 insertions(+), 58 deletions(-) diff --git a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift index c8f8b2584..7d1d132bd 100644 --- a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift +++ b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift @@ -138,7 +138,7 @@ fileprivate extension LibSessionUtilSpec { it("can catch size limit errors thrown when pushing") { var randomGenerator: ARC4RandomNumberGenerator = ARC4RandomNumberGenerator(seed: 1000) - try (0..<10000).forEach { index in + try (0..<2500).forEach { index in var contact: contacts_contact = try createContact( for: index, in: conf, @@ -148,7 +148,7 @@ fileprivate extension LibSessionUtilSpec { contacts_set(conf, &contact) } - expect(contacts_size(conf)).to(equal(10000)) + expect(contacts_size(conf)).to(equal(2500)) expect(config_needs_push(conf)).to(beTrue()) expect(config_needs_dump(conf)).to(beTrue()) @@ -187,109 +187,77 @@ fileprivate extension LibSessionUtilSpec { // MARK: ---- has not changed the max empty records it("has not changed the max empty records") { var randomGenerator: ARC4RandomNumberGenerator = ARC4RandomNumberGenerator(seed: 1000) + let expectedRecords: Int = 2212 - for index in (0..<100000) { + repeat { var contact: contacts_contact = try createContact( - for: index, + for: numRecords, in: conf, rand: &randomGenerator ) contacts_set(conf, &contact) - - do { - config_push(conf)?.deallocate() - try LibSessionError.throwIfNeeded(conf) - } - catch { break } - - // We successfully inserted a contact and didn't hit the limit so increment the counter - numRecords += 1 - } + } while !has(conf, with: &numRecords, hitLimit: expectedRecords) // Check that the record count matches the maximum when we last checked - expect(numRecords).to(equal(2212)) + expect(numRecords).to(equal(expectedRecords)) } // MARK: ---- has not changed the max name only records it("has not changed the max name only records") { var randomGenerator: ARC4RandomNumberGenerator = ARC4RandomNumberGenerator(seed: 1000) + let expectedRecords: Int = 742 - for index in (0..<100000) { + repeat { var contact: contacts_contact = try createContact( - for: index, + for: numRecords, in: conf, rand: &randomGenerator, maxing: [.name] ) contacts_set(conf, &contact) - - do { - config_push(conf)?.deallocate() - try LibSessionError.throwIfNeeded(conf) - } - catch { break } - - // We successfully inserted a contact and didn't hit the limit so increment the counter - numRecords += 1 - } + } while !has(conf, with: &numRecords, hitLimit: expectedRecords) // Check that the record count matches the maximum when we last checked - expect(numRecords).to(equal(742)) + expect(numRecords).to(equal(expectedRecords)) } // MARK: ---- has not changed the max name and profile pic only records it("has not changed the max name and profile pic only records") { var randomGenerator: ARC4RandomNumberGenerator = ARC4RandomNumberGenerator(seed: 1000) + let expectedRecords: Int = 274 - for index in (0..<100000) { + repeat { var contact: contacts_contact = try createContact( - for: index, + for: numRecords, in: conf, rand: &randomGenerator, maxing: [.name, .profile_pic] ) contacts_set(conf, &contact) - - do { - config_push(conf)?.deallocate() - try LibSessionError.throwIfNeeded(conf) - } - catch { break } - - // We successfully inserted a contact and didn't hit the limit so increment the counter - numRecords += 1 - } + } while !has(conf, with: &numRecords, hitLimit: expectedRecords) // Check that the record count matches the maximum when we last checked - expect(numRecords).to(equal(274)) + expect(numRecords).to(equal(expectedRecords)) } // MARK: ---- has not changed the max filled records it("has not changed the max filled records") { var randomGenerator: ARC4RandomNumberGenerator = ARC4RandomNumberGenerator(seed: 1000) + let expectedRecords: [Int] = [222, 223] - for index in (0..<100000) { + repeat { var contact: contacts_contact = try createContact( - for: index, + for: numRecords, in: conf, rand: &randomGenerator, maxing: .allProperties ) contacts_set(conf, &contact) - - do { - config_push(conf)?.deallocate() - try LibSessionError.throwIfNeeded(conf) - } - catch { break } - - // We successfully inserted a contact and didn't hit the limit so increment the counter - numRecords += 1 - } + } while !has(conf, with: &numRecords, hitLimit: expectedRecords.max()!) // Check that the record count matches the maximum when we last checked (seems to swap between // these two on different test runs for some reason) - expect(numRecords).to(satisfyAnyOf(equal(222), equal(223))) + expect(numRecords).to(satisfyAnyOf(expectedRecords.map { equal($0) })) } } @@ -512,8 +480,10 @@ fileprivate extension LibSessionUtilSpec { let pushData7: UnsafeMutablePointer = config_push(conf2) expect(pushData7.pointee.seqno).to(equal(3)) - let pushData6Str: String = String(pointer: pushData6.pointee.config, length: pushData6.pointee.config_len, encoding: .ascii)! - let pushData7Str: String = String(pointer: pushData7.pointee.config, length: pushData7.pointee.config_len, encoding: .ascii)! + let pushData6Str: String? = String(pointer: pushData6.pointee.config, length: pushData6.pointee.config_len, encoding: .ascii) + let pushData7Str: String? = String(pointer: pushData7.pointee.config, length: pushData7.pointee.config_len, encoding: .ascii) + expect(pushData6Str).toNot(beNil()) + expect(pushData7Str).toNot(beNil()) expect(pushData6Str).toNot(equal(pushData7Str)) expect([String](pointer: pushData6.pointee.obsolete, count: pushData6.pointee.obsolete_len)) .to(equal([fakeHash2])) @@ -555,8 +525,10 @@ fileprivate extension LibSessionUtilSpec { let pushData9: UnsafeMutablePointer = config_push(conf2) expect(pushData9.pointee.seqno).to(equal(pushData8.pointee.seqno)) - let pushData8Str: String = String(pointer: pushData8.pointee.config, length: pushData8.pointee.config_len, encoding: .ascii)! - let pushData9Str: String = String(pointer: pushData9.pointee.config, length: pushData9.pointee.config_len, encoding: .ascii)! + let pushData8Str: String? = String(pointer: pushData8.pointee.config, length: pushData8.pointee.config_len, encoding: .ascii) + let pushData9Str: String = String(pointer: pushData9.pointee.config, length: pushData9.pointee.config_len, encoding: .ascii)? + expect(pushData8Str).toNot(beNil()) + expect(pushData9Str).toNot(beNil()) expect(pushData8Str).to(equal(pushData9Str)) expect([String](pointer: pushData8.pointee.obsolete, count: pushData8.pointee.obsolete_len)) .to(equal([fakeHash3b, fakeHash3a])) @@ -1723,3 +1695,62 @@ fileprivate extension LibSessionUtilSpec { } } } + +// MARK: - Convenience + +private extension LibSessionUtilSpec { + static func has(_ conf: UnsafeMutablePointer?, with numRecords: inout Int, hitLimit expectedLimit: Int) -> Bool { + // Have a hard limit (ie. don't want to loop over this limit as it likely means something is busted elsewhere + // and we are in an infinite loop) + guard numRecords < 2500 else { return true } + + // When generating push data the actual data generated is based on a diff from the current state to the + // next state - this means that adding 100 records at once is a different size from adding 1 at a time, + // but since adding them 1 at a time is really inefficient we want to try to be smart about calling + // `config_push` when we are far away from the limit, but do so in such a way that we still get accurate + // sizes as we approach the limit (this includes the "diff" values which include the last 5 changes) + // + // **Note:** `config_push` returns null when it hits the config limit + let distanceToLimit: Int = (expectedLimit - numRecords) + + switch distanceToLimit { + case Int.min...50: + // Within 50 records of the expected limit we want to check every record + guard let result: UnsafeMutablePointer = config_push(conf) else { return true } + + // We successfully generated the config push and didn't hit the limit + result.deallocate() + + case 50...100: + // Between 50 and 100 records of the expected limit only check every `10` records + if numRecords.isMultiple(of: 10) { + guard let result: UnsafeMutablePointer = config_push(conf) else { return true } + + // We successfully generated the config push and didn't hit the limit + result.deallocate() + } + + case 100...200: + // Between 100 and 200 records of the expected limit only check every `25` records + if numRecords.isMultiple(of: 25) { + guard let result: UnsafeMutablePointer = config_push(conf) else { return true } + + // We successfully generated the config push and didn't hit the limit + result.deallocate() + } + + default: + // Otherwise check every `50` records + if numRecords.isMultiple(of: 50) { + guard let result: UnsafeMutablePointer = config_push(conf) else { return true } + + // We successfully generated the config push and didn't hit the limit + result.deallocate() + } + } + + // Increment the number of records + numRecords += 1 + return false + } +} From afc289808f091e41f5d00cc13c173d3fad2832e7 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 9 Oct 2024 13:58:50 +1100 Subject: [PATCH 2/8] Fixed a silly typo --- SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift index 7d1d132bd..1b1019540 100644 --- a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift +++ b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift @@ -526,7 +526,7 @@ fileprivate extension LibSessionUtilSpec { expect(pushData9.pointee.seqno).to(equal(pushData8.pointee.seqno)) let pushData8Str: String? = String(pointer: pushData8.pointee.config, length: pushData8.pointee.config_len, encoding: .ascii) - let pushData9Str: String = String(pointer: pushData9.pointee.config, length: pushData9.pointee.config_len, encoding: .ascii)? + let pushData9Str: String? = String(pointer: pushData9.pointee.config, length: pushData9.pointee.config_len, encoding: .ascii) expect(pushData8Str).toNot(beNil()) expect(pushData9Str).toNot(beNil()) expect(pushData8Str).to(equal(pushData9Str)) From 3241e6c529081af9f4f3f9420ce31e7d6dac89d9 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 9 Oct 2024 14:27:13 +1100 Subject: [PATCH 3/8] Fixed some test failure due to incorrect type conversions --- .../LibSession/LibSessionUtilSpec.swift | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift index 1b1019540..dea81913f 100644 --- a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift +++ b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift @@ -480,11 +480,9 @@ fileprivate extension LibSessionUtilSpec { let pushData7: UnsafeMutablePointer = config_push(conf2) expect(pushData7.pointee.seqno).to(equal(3)) - let pushData6Str: String? = String(pointer: pushData6.pointee.config, length: pushData6.pointee.config_len, encoding: .ascii) - let pushData7Str: String? = String(pointer: pushData7.pointee.config, length: pushData7.pointee.config_len, encoding: .ascii) - expect(pushData6Str).toNot(beNil()) - expect(pushData7Str).toNot(beNil()) - expect(pushData6Str).toNot(equal(pushData7Str)) + let pushData6Data: Data = Data(bytes: pushData6.pointee.config, count: pushData6.pointee.config_len) + let pushData7Data: Data = Data(bytes: pushData7.pointee.config, count: pushData7.pointee.config_len) + expect(pushData6Data).toNot(equal(pushData7Data)) expect([String](pointer: pushData6.pointee.obsolete, count: pushData6.pointee.obsolete_len)) .to(equal([fakeHash2])) expect([String](pointer: pushData7.pointee.obsolete, count: pushData7.pointee.obsolete_len)) @@ -525,11 +523,9 @@ fileprivate extension LibSessionUtilSpec { let pushData9: UnsafeMutablePointer = config_push(conf2) expect(pushData9.pointee.seqno).to(equal(pushData8.pointee.seqno)) - let pushData8Str: String? = String(pointer: pushData8.pointee.config, length: pushData8.pointee.config_len, encoding: .ascii) - let pushData9Str: String? = String(pointer: pushData9.pointee.config, length: pushData9.pointee.config_len, encoding: .ascii) - expect(pushData8Str).toNot(beNil()) - expect(pushData9Str).toNot(beNil()) - expect(pushData8Str).to(equal(pushData9Str)) + let pushData8Data: Data = Data(bytes: pushData8.pointee.config, count: pushData8.pointee.config_len) + let pushData9Data: Data = Data(bytes: pushData9.pointee.config, count: pushData9.pointee.config_len) + expect(pushData8Data).to(equal(pushData9Data)) expect([String](pointer: pushData8.pointee.obsolete, count: pushData8.pointee.obsolete_len)) .to(equal([fakeHash3b, fakeHash3a])) expect([String](pointer: pushData9.pointee.obsolete, count: pushData9.pointee.obsolete_len)) @@ -818,9 +814,9 @@ fileprivate extension LibSessionUtilSpec { // Since we set different things, we're going to get back different serialized data to be // pushed: - let pushData3Str: String? = String(pointer: pushData3.pointee.config, length: pushData3.pointee.config_len, encoding: .ascii) - let pushData4Str: String? = String(pointer: pushData4.pointee.config, length: pushData4.pointee.config_len, encoding: .ascii) - expect(pushData3Str).toNot(equal(pushData4Str)) + let pushData3Data: Data = Data(bytes: pushData3.pointee.config, count: pushData3.pointee.config_len) + let pushData4Data: Data = Data(bytes: pushData4.pointee.config, count: pushData4.pointee.config_len) + expect(pushData3Data).toNot(equal(pushData4Data)) // Now imagine that each client pushed its `seqno=2` config to the swarm, but then each client // also fetches new messages and pulls down the other client's `seqno=2` value. From c693a204620c3ba93462be3b646c8b9aec063fcd Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 10 Oct 2024 10:30:35 +1100 Subject: [PATCH 4/8] Debugging CI 1 --- .drone.jsonnet | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.drone.jsonnet b/.drone.jsonnet index 51ab75b2a..b00f1f3ec 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -76,6 +76,7 @@ local sim_delete_cmd = 'if [ -f build/artifacts/sim_uuid ]; then rm -f /Users/$U commands: [ sim_delete_cmd, ||| + which xcresultparser if [[ -d ./build/artifacts/testResults.xcresult ]]; then xcresultparser --output-format cli --failed-tests-only ./build/artifacts/testResults.xcresult else @@ -91,6 +92,7 @@ local sim_delete_cmd = 'if [ -f build/artifacts/sim_uuid ]; then rm -f /Users/$U { name: 'Convert xcresult to xml', commands: [ + 'which xcresultparser', 'xcresultparser --output-format cobertura ./build/artifacts/testResults.xcresult > ./build/artifacts/coverage.xml', ], depends_on: ['Build and Run Tests'], From 646fc2359de4c2ba7b544ca36a6744e21db0e414 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 10 Oct 2024 10:31:09 +1100 Subject: [PATCH 5/8] Debugging CI 2 --- .drone.jsonnet | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index b00f1f3ec..324360691 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -6,6 +6,7 @@ local version_info = { name: 'Version Information', environment: { LANG: 'en_US.UTF-8' }, commands: [ + 'which xcresultparser', 'git --version', 'xcodebuild -version', 'xcbeautify --version', @@ -76,7 +77,6 @@ local sim_delete_cmd = 'if [ -f build/artifacts/sim_uuid ]; then rm -f /Users/$U commands: [ sim_delete_cmd, ||| - which xcresultparser if [[ -d ./build/artifacts/testResults.xcresult ]]; then xcresultparser --output-format cli --failed-tests-only ./build/artifacts/testResults.xcresult else @@ -92,7 +92,6 @@ local sim_delete_cmd = 'if [ -f build/artifacts/sim_uuid ]; then rm -f /Users/$U { name: 'Convert xcresult to xml', commands: [ - 'which xcresultparser', 'xcresultparser --output-format cobertura ./build/artifacts/testResults.xcresult > ./build/artifacts/coverage.xml', ], depends_on: ['Build and Run Tests'], From 8e0fabed17df762a2e1249c86e4d22930568848b Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 10 Oct 2024 10:40:28 +1100 Subject: [PATCH 6/8] Removing debug CI stuff --- .drone.jsonnet | 1 - 1 file changed, 1 deletion(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index 324360691..51ab75b2a 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -6,7 +6,6 @@ local version_info = { name: 'Version Information', environment: { LANG: 'en_US.UTF-8' }, commands: [ - 'which xcresultparser', 'git --version', 'xcodebuild -version', 'xcbeautify --version', From 0b97122209f4bd4cee3c9f272d225f40f5f4d029 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 10 Oct 2024 11:38:24 +1100 Subject: [PATCH 7/8] Try to clean up CI output a little --- .drone.jsonnet | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.drone.jsonnet b/.drone.jsonnet index 51ab75b2a..6c7d7eb2c 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -76,6 +76,8 @@ local sim_delete_cmd = 'if [ -f build/artifacts/sim_uuid ]; then rm -f /Users/$U commands: [ sim_delete_cmd, ||| + set +x + if [[ -d ./build/artifacts/testResults.xcresult ]]; then xcresultparser --output-format cli --failed-tests-only ./build/artifacts/testResults.xcresult else From dae4715ca5dff901e6468bdff578d2a67fce4a06 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 10 Oct 2024 12:03:42 +1100 Subject: [PATCH 8/8] Tweaked the unit test summary step to be simpler --- .drone.jsonnet | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index 6c7d7eb2c..6163e4f0f 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -75,20 +75,9 @@ local sim_delete_cmd = 'if [ -f build/artifacts/sim_uuid ]; then rm -f /Users/$U name: 'Unit Test Summary', commands: [ sim_delete_cmd, - ||| - set +x - - if [[ -d ./build/artifacts/testResults.xcresult ]]; then - xcresultparser --output-format cli --failed-tests-only ./build/artifacts/testResults.xcresult - else - echo -e "\n\n\n\e[31;1mUnit test results not found\e[0m" - fi - |||, + 'xcresultparser --output-format cli --failed-tests-only ./build/artifacts/testResults.xcresult' ], - depends_on: ['Build and Run Tests'], - when: { - status: ['failure', 'success'], - }, + depends_on: ['Build and Run Tests'] }, { name: 'Convert xcresult to xml',