Skip to content

Remove unneeded @testable imports#670

Open
shyamkumar703 wants to merge 4 commits intoxmtp:mainfrom
shyamkumar703:shyam/remove-unneeded-testable-imports
Open

Remove unneeded @testable imports#670
shyamkumar703 wants to merge 4 commits intoxmtp:mainfrom
shyamkumar703:shyam/remove-unneeded-testable-imports

Conversation

@shyamkumar703
Copy link

Introduction 📟

Replaces @testable import XMTPiOS with import XMTPiOS where possible; so tests are testing the public SDK API instead of internal fields/functions

Purpose ℹ️

Test the public API instead of internal fields/functions

Scope 🔭

Test cases

@shyamkumar703 shyamkumar703 requested a review from a team as a code owner February 20, 2026 17:03
@shyamkumar703
Copy link
Author

Proposed fix for #373

@macroscopeapp
Copy link

macroscopeapp bot commented Feb 20, 2026

Remove @testable imports from XMTP tests and switch test key generation to XMTPTestHelpers.TestHelpers.generateRandomBytes(count: 32)

Add TestHelpers.generateRandomBytes and a Date milliseconds utility in TestHelpers.swift; update tests to import XMTPiOS and XMTPTestHelpers and replace Crypto.secureRandomBytes with generateRandomBytes for 32‑byte keys.

📍Where to Start

Start with the new helpers in TestHelpers.swift, then review a representative test update such as ArchiveTests.swift to see import and key generation changes.


📊 Macroscope summarized 6b1016e. 1 file reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted. View details

}

// INFO: - copied from Date.swift
public extension Date {
Copy link
Author

Choose a reason for hiding this comment

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

Copied wholesale from Date.swift - a Common module with shared utilities across the test/source targets might be a better pattern here but seems overkill for just this extension.

}

init(millisecondsSinceEpoch: Int64) {
self.init(timeIntervalSince1970: TimeInterval(millisecondsSinceEpoch / 1_000_000_000))
Copy link

Choose a reason for hiding this comment

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

🟢 Low

XMTPTestHelpers/TestHelpers.swift:119 The divisor 1_000_000_000 converts nanoseconds, not milliseconds. Consider using 1_000 to match the parameter name.

Suggested change
self.init(timeIntervalSince1970: TimeInterval(millisecondsSinceEpoch / 1_000_000_000))
self.init(timeIntervalSince1970: TimeInterval(millisecondsSinceEpoch / 1_000))

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI
In file Sources/XMTPTestHelpers/TestHelpers.swift around line 119:

The divisor `1_000_000_000` converts nanoseconds, not milliseconds. Consider using `1_000` to match the parameter name.

Evidence trail:
Sources/XMTPTestHelpers/TestHelpers.swift lines 117-120 at commit 6b1016e1e20f5cc786b57d6aafaad72bfb94c789: `init(millisecondsSinceEpoch: Int64) { self.init(timeIntervalSince1970: TimeInterval(millisecondsSinceEpoch / 1_000_000_000)) }`. The parameter is named `millisecondsSinceEpoch` but the divisor 1_000_000_000 converts nanoseconds to seconds, not milliseconds. Milliseconds to seconds conversion requires dividing by 1_000.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant