Skip to content

Commit

Permalink
Tracing and integration test tweaks (#3336)
Browse files Browse the repository at this point in the history
* Disable image and document picker integration tests as they randomly fail to load and are flakey.

* Delete any pre-existing log files

* Various tracing tweaks and fixes:
- delete the custom tracing log levels as we can't control their ouput
- implement comparable on them
- change default levels only if the new chosen level increases their verbosity

* Make logging targets mandatory and fix their logging levels

* Switch back to using the `run_tests` reset simulator flag as `fastlane snapshot reset_simulators` was too generic and slow

* Switch all integration test taps to `tapCenter` (nee forceTap) after noticing missed taps on CI.

* Make the logging file prefix explicit, let the main app not use one.

* Rename tracing configuration `target` to `currentTarget`
  • Loading branch information
stefanceriu authored Sep 27, 2024
1 parent df6d0a9 commit 9d23dec
Show file tree
Hide file tree
Showing 16 changed files with 127 additions and 139 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ jobs:
run:
source ci_scripts/ci_common.sh && setup_github_actions_environment

- name: Reset simulators
run: SNAPSHOT_FORCE_DELETE=1 bundle exec fastlane snapshot reset_simulators
- name: Delete old log files
run: find '/Users/Shared' -name 'console*' -delete

- name: Run tests
run: bundle exec fastlane integration_tests
Expand All @@ -44,7 +44,7 @@ jobs:
run: (grep ' TRACE ' /Users/Shared -qR)

- name: Check logs don't contain private messages
run: (! grep 'Go down in flames' /Users/Shared -qR)
run: "! grep 'Go down in flames' /Users/Shared -R"

- name: Zip results # for faster upload
if: failure()
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ jobs:
- name: SwiftFormat
run:
swiftformat --lint .

- name: Reset simulators
run: SNAPSHOT_FORCE_DELETE=1 bundle exec fastlane snapshot reset_simulators

- name: Run tests
run: bundle exec fastlane unit_tests
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/unit_tests_enterprise.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ jobs:
- name: SwiftFormat
run: swiftformat --lint .

- name: Reset simulators
run: SNAPSHOT_FORCE_DELETE=1 bundle exec fastlane snapshot reset_simulators

- name: Run tests
run: bundle exec fastlane unit_tests skip_previews:true

Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/Application/AppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg

let appSettings = appHooks.appSettingsHook.configure(AppSettings())

MXLog.configure(logLevel: appSettings.logLevel)
MXLog.configure(currentTarget: "elementx", filePrefix: nil, logLevel: appSettings.logLevel)

let appName = InfoPlistReader.main.bundleDisplayName
let appVersion = InfoPlistReader.main.bundleShortVersionString
Expand Down
4 changes: 2 additions & 2 deletions ElementX/Sources/Other/Extensions/XCUIElement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import XCTest

extension XCUIElement {
func clearAndTypeText(_ text: String) {
forceTap()
tapCenter()

guard let currentValue = value as? String else {
XCTFail("Tried to clear and type text into a non string value")
Expand All @@ -24,7 +24,7 @@ extension XCUIElement {
}
}

func forceTap() {
func tapCenter() {
let coordinate: XCUICoordinate = coordinate(withNormalizedOffset: .init(dx: 0.5, dy: 0.5))
coordinate.tap()
}
Expand Down
19 changes: 8 additions & 11 deletions ElementX/Sources/Other/Logging/MXLog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,18 @@ enum MXLog {
private static var didConfigureOnce = false

private static var rootSpan: Span!
private static var target: String!
private static var currentTarget: String!

static func configure(target: String? = nil,
static func configure(currentTarget: String,
filePrefix: String?,
logLevel: TracingConfiguration.LogLevel) {
guard !didConfigureOnce else { return }

RustTracing.setup(configuration: .init(logLevel: logLevel, target: target))
RustTracing.setup(configuration: .init(logLevel: logLevel, currentTarget: currentTarget, filePrefix: filePrefix))

if let target {
self.target = target
} else {
self.target = Constants.target
}
self.currentTarget = currentTarget

rootSpan = Span(file: #file, line: #line, level: .info, target: self.target, name: "root")
rootSpan = Span(file: #file, line: #line, level: .info, target: self.currentTarget, name: "root")
rootSpan.enter()

didConfigureOnce = true
Expand Down Expand Up @@ -138,7 +135,7 @@ enum MXLog {
rootSpan.enter()
}

return Span(file: file, line: UInt32(line), level: level, target: target, name: name)
return Span(file: file, line: UInt32(line), level: level, target: currentTarget, name: name)
}

// periphery:ignore:parameters function,column,context
Expand All @@ -157,6 +154,6 @@ enum MXLog {
rootSpan.enter()
}

logEvent(file: (file as NSString).lastPathComponent, line: UInt32(line), level: level, target: target, message: "\(message)")
logEvent(file: (file as NSString).lastPathComponent, line: UInt32(line), level: level, target: currentTarget, message: "\(message)")
}
}
2 changes: 1 addition & 1 deletion ElementX/Sources/Other/Logging/RustTracing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ enum RustTracing {
// Log everything on integration tests to check whether
// the logs contain any sensitive data. See `UserFlowTests.swift`
let filter = if ProcessInfo.isRunningIntegrationTests {
TracingConfiguration(logLevel: .trace, target: nil).filter
TracingConfiguration(logLevel: .trace, currentTarget: "integrationtests", filePrefix: nil).filter
} else {
configuration.filter
}
Expand Down
83 changes: 40 additions & 43 deletions ElementX/Sources/Other/Logging/TracingConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ import Collections
// We can filter by level, crate and even file. See more details here:
// https://docs.rs/tracing-subscriber/0.2.7/tracing_subscriber/filter/struct.EnvFilter.html#examples
struct TracingConfiguration {
enum LogLevel: Codable, Hashable {
enum LogLevel: String, Codable, Hashable, Comparable {
case error, warn, info, debug, trace
case custom(String)

var title: String {
switch self {
Expand All @@ -27,34 +26,32 @@ struct TracingConfiguration {
return "Debug"
case .trace:
return "Trace"
case .custom:
return "Custom"
}
}

fileprivate var rawValue: String {
switch self {
case .error:
return "error"
case .warn:
return "warn"
case .info:
return "info"
case .debug:
return "debug"
case .trace:
return "trace"
case .custom(let filter):
return filter
static func < (lhs: TracingConfiguration.LogLevel, rhs: TracingConfiguration.LogLevel) -> Bool {
switch (lhs, rhs) {
case (.error, _):
true
case (.warn, .error):
false
case (.warn, _):
true
case (.info, .error), (.info, .warn):
false
case (.info, _):
true
case (.debug, .error), (.debug, .warn), (.debug, .info):
false
case (.debug, _):
true
case (.trace, _):
false
}
}
}

enum Target: String {
case common = ""

case elementx

case hyper, matrix_sdk_ffi, matrix_sdk_crypto

case matrix_sdk_client = "matrix_sdk::client"
Expand All @@ -66,9 +63,8 @@ struct TracingConfiguration {
case matrix_sdk_ui_timeline = "matrix_sdk_ui::timeline"
}

// The `common` target is excluded because 3rd-party crates might end up logging user data.
static let targets: OrderedDictionary<Target, LogLevel> = [
.common: .info, // Never set this lower than info - 3rd-party crates may start logging user data.
.elementx: .info,
.hyper: .warn,
.matrix_sdk_ffi: .info,
.matrix_sdk_client: .trace,
Expand All @@ -92,48 +88,49 @@ struct TracingConfiguration {
/// - Parameter logLevel: the desired log level
/// - Parameter target: the name of the target being configured
/// - Returns: a custom tracing configuration
init(logLevel: LogLevel, target: String?) {
fileName = if let target {
"\(RustTracing.filePrefix)-\(target)"
init(logLevel: LogLevel, currentTarget: String, filePrefix: String?) {
fileName = if let filePrefix {
"\(RustTracing.filePrefix)-\(filePrefix)"
} else {
RustTracing.filePrefix
}

if case let .custom(filter) = logLevel {
self.filter = filter
return
}


let overrides = Self.targets.keys.reduce(into: [Target: LogLevel]()) { partialResult, target in
// Keep the defaults here
let ignoredTargets: [Target] = [.common, // Never remove common from the ignored targets (see above for more info).
.hyper,
.matrix_sdk_ffi,
.matrix_sdk_oidc,
.matrix_sdk_client,
.matrix_sdk_crypto,
.matrix_sdk_crypto_account,
.matrix_sdk_http_client]
let ignoredTargets: [Target] = [.hyper]

if ignoredTargets.contains(target) {
return
}

partialResult[target] = logLevel
guard let defaultTargetLogLevel = Self.targets[target] else {
return
}

// Only change the targets that have default values
// smaller than the desired log level
if defaultTargetLogLevel < logLevel {
partialResult[target] = logLevel
}
}

var newTargets = Self.targets
for (target, logLevel) in overrides {
newTargets.updateValue(logLevel, forKey: target)
}

let components = newTargets.map { (target: Target, logLevel: LogLevel) in
var components = newTargets.map { (target: Target, logLevel: LogLevel) in
guard !target.rawValue.isEmpty else {
return logLevel.rawValue
}

return "\(target.rawValue)=\(logLevel.rawValue)"
}

// With `common` not being used we manually need to specify the log
// level for passed in targets
components.append("\(currentTarget)=\(logLevel.rawValue)")

filter = components.joined(separator: ",")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,6 @@ struct DeveloperOptionsScreen: View {
private struct LogLevelConfigurationView: View {
@Binding var logLevel: TracingConfiguration.LogLevel

@State private var customTracingConfiguration: String

init(logLevel: Binding<TracingConfiguration.LogLevel>) {
_logLevel = logLevel

if case .custom(let configuration) = logLevel.wrappedValue {
customTracingConfiguration = configuration
} else {
customTracingConfiguration = TracingConfiguration(logLevel: .info, target: nil).filter
}
}

var body: some View {
Picker(selection: $logLevel) {
ForEach(logLevels, id: \.self) { logLevel in
Expand All @@ -156,24 +144,11 @@ private struct LogLevelConfigurationView: View {
Text("Log level")
Text("Requires app reboot")
}

if case .custom = logLevel {
TextEditor(text: $customTracingConfiguration)
.textInputAutocapitalization(.never)
.autocorrectionDisabled()
.onChange(of: customTracingConfiguration) { newValue in
logLevel = .custom(newValue)
}
}
}

/// Allows the picker to work with associated values
private var logLevels: [TracingConfiguration.LogLevel] {
if case let .custom(filter) = logLevel {
return [.error, .warn, .info, .debug, .trace, .custom(filter)]
} else {
return [.error, .warn, .info, .debug, .trace, .custom("")]
}
[.error, .warn, .info, .debug, .trace]
}
}

Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/UITests/UITestsAppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class UITestsAppCoordinator: AppCoordinatorProtocol, SecureWindowManagerDelegate

windowManager.delegate = self

MXLog.configure(logLevel: .debug)
MXLog.configure(currentTarget: "uitests", filePrefix: nil, logLevel: .debug)

ServiceLocator.shared.register(userIndicatorController: UserIndicatorController())

Expand Down
21 changes: 10 additions & 11 deletions IntegrationTests/Sources/Common.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ extension XCUIApplication {
let getStartedButton = buttons[A11yIdentifiers.authenticationStartScreen.signIn]

XCTAssertTrue(getStartedButton.waitForExistence(timeout: 10.0))
getStartedButton.tap()
getStartedButton.tapCenter()

// Get started is network bound, wait for the change homeserver button for longer
let changeHomeserverButton = buttons[A11yIdentifiers.serverConfirmationScreen.changeServer]
XCTAssertTrue(changeHomeserverButton.waitForExistence(timeout: 30.0))
changeHomeserverButton.tap()
XCTAssertTrue(changeHomeserverButton.waitForExistence(timeout: 10.0))
changeHomeserverButton.tapCenter()

let homeserverTextField = textFields[A11yIdentifiers.changeServerScreen.server]
XCTAssertTrue(homeserverTextField.waitForExistence(timeout: 10.0))
Expand All @@ -26,7 +25,7 @@ extension XCUIApplication {

let confirmButton = buttons[A11yIdentifiers.changeServerScreen.continue]
XCTAssertTrue(confirmButton.waitForExistence(timeout: 10.0))
confirmButton.tap()
confirmButton.tapCenter()

// Wait for server confirmation to finish
let doesNotExistPredicate = NSPredicate(format: "exists == 0")
Expand All @@ -35,7 +34,7 @@ extension XCUIApplication {

let continueButton = buttons[A11yIdentifiers.serverConfirmationScreen.continue]
XCTAssertTrue(continueButton.waitForExistence(timeout: 30.0))
continueButton.tap()
continueButton.tapCenter()

let usernameTextField = textFields[A11yIdentifiers.loginScreen.emailUsername]
XCTAssertTrue(usernameTextField.waitForExistence(timeout: 10.0))
Expand All @@ -51,7 +50,7 @@ extension XCUIApplication {
XCTAssertTrue(nextButton.waitForExistence(timeout: 10.0))
XCTAssertTrue(nextButton.isEnabled)

nextButton.tap()
nextButton.tapCenter()

// Wait for login to finish
currentTestCase.expectation(for: doesNotExistPredicate, evaluatedWith: usernameTextField)
Expand All @@ -63,7 +62,7 @@ extension XCUIApplication {
// Tapping the sheet button while animating upwards fails. Wait for it to settle
sleep(1)

savePasswordButton.tap()
savePasswordButton.tapCenter()
}

// Wait for the home screen to become visible.
Expand All @@ -80,17 +79,17 @@ extension XCUIApplication {
let profileButton = buttons[A11yIdentifiers.homeScreen.userAvatar]

// `Failed to scroll to visible (by AX action) Button` https://stackoverflow.com/a/33534187/730924
profileButton.forceTap()
profileButton.tapCenter()

// Logout
let logoutButton = buttons[A11yIdentifiers.settingsScreen.logout]
XCTAssertTrue(logoutButton.waitForExistence(timeout: 10.0))
logoutButton.tap()
logoutButton.tapCenter()

// Confirm logout
let alertLogoutButton = alerts.firstMatch.buttons["Sign out"]
XCTAssertTrue(alertLogoutButton.waitForExistence(timeout: 10.0))
alertLogoutButton.tap()
alertLogoutButton.tapCenter()

// Check that we're back on the login screen
let getStartedButton = buttons[A11yIdentifiers.authenticationStartScreen.signIn]
Expand Down
Loading

0 comments on commit 9d23dec

Please sign in to comment.