[XCUITests] Initial tests for Activity Indicator #1458

joannaquu · 2022-12-16T19:40:59Z

Platforms Impacted

iOS
macOS

Description of changes

Binary change:

File	Before	After	Delta
libFluentUI.a	54,943,520 bytes	54,965,576 bytes	22,056 bytes

Added initial tests for activity indicator. These include tests for size, color, and starting/stopping activity (both when hidden when stopped is turned on and off).

Verification

UIKit	SwiftUI

Pull request checklist

This PR has considered:

Light and Dark appearances
iOS supported versions (all major versions greater than or equal current target deployment version)
VoiceOver and Keyboard Accessibility
Internationalization and Right to Left layouts
Different resolutions (1x, 2x, 3x)
Size classes and window sizes (iPhone vs iPad, notched devices, multitasking, different window sizes, etc)
iPad Pointer interaction
SwiftUI consumption (validation or new demo scenarios needed)
Objective-C exposure (provide it only if needed)

Microsoft Reviewers: Open in CodeFlow

ios/FluentUI/ActivityIndicator/ActivityIndicatorTokenSet.swift

laminesm

We use lint to enforce some stylistic conventions. I got it with homebrew (https://formulae.brew.sh/formula/swiftlint) and you can run it in the terminal before pushing PRs and it'll fix the warnings for you :)

mischreiber · 2022-12-16T23:33:32Z

ios/FluentUI.Demo/FluentUIDemoTests/ActivityIndicatorTest.swift

+    }
+
+    func testSizes() throws {
+        XCTAssertEqual(app.images.element(boundBy: 0).identifier, "Activity Indicator that is In progress of size 4")


Interesting note: these tests fail if your simulator is set to any language other than en-US. Are we okay with that?

I won't have access to language inside each test so I think we might have to be if we want any sort of ui tests but I am open to any suggestions.

I think any tests we write should eventually work for any language we configure the tests to run on. Accessibility label/accessibility names both get localized, can we build the accessibility identifier off of something else?

Looks like the accessibility label is created from the isAnimating property on the activity indicator, maybe we can use that directly. Color seems a little more tricky - I wonder if we can use the RGB value

lyzhan7 · 2022-12-19T19:18:25Z

ios/FluentUI.Demo/FluentUIDemoTests/ActivityIndicatorTest.swift

+        if app.staticTexts["FluentUI DEV"].exists {
+            app.staticTexts["ActivityIndicator"].tap()
+        } else if !app.staticTexts["ActivityIndicator"].exists {
+            app.buttons["FluentUI DEV"].tap()


Hmm is there a button called FluentUI Dev?

There's no label for the back button so it defaulted to FluentUI Dev. I wasn't sure if I should go in and change it so I left if for now.

lyzhan7 · 2022-12-19T19:20:01Z

ios/FluentUI.Demo/FluentUIDemoTests/ActivityIndicatorTest.swift

+        continueAfterFailure = false
+        app.launch()
+
+        if app.staticTexts["FluentUI DEV"].exists {


I think we might want to consider setting up "FluentUI DEV" and "ActivityIndicator" as constants, as well as some of the controls we're testing ex. "app.switches["Animating"]" maybe could be defined once

lyzhan7 · 2022-12-19T20:02:55Z

ios/FluentUI.Demo/FluentUIDemoTests/ActivityIndicatorTest.swift

+    }
+
+    func testSizes() throws {
+        XCTAssertEqual(app.images.element(boundBy: 0).identifier, "Activity Indicator that is In progress of size 4")


I think any tests we write should eventually work for any language we configure the tests to run on. Accessibility label/accessibility names both get localized, can we build the accessibility identifier off of something else?

Looks like the accessibility label is created from the isAnimating property on the activity indicator, maybe we can use that directly. Color seems a little more tricky - I wonder if we can use the RGB value

lyzhan7 · 2022-12-19T20:41:31Z

Some overall notes:

What's getting tested
- It looks like this PR is pretty much testing that every single variation of the activity indicator exists - it's great how thorough this is, but not sure how feasible it is to do this for every component. There are a couple components that have a ton of variations (ex. Avatar), that might take up a lot of time to do this for- it might be more impactful to first establish a simpler baseline of tests across all the components first (ex. just opening every test page for every component, and then maybe checking that one component on every page exists), rather than potentially getting hung up on the components that have a ton of variants. This is also the approach that the FURN tests have taken.
In your PR description I think you could add a bit more context on what these changes do:
- You mentioned adding "initial tests" for activity indicator - can you add more detail to what the tests are testing and what the reasoning is behind them?
- And related to the above - what is in scope for this PR and what is out of scope? Ex. something out of scope - I would expect the UI tests to eventually be testing more behavior - ex. does toggling the "animating" button actually stop/start the animation?
- Optional - since XCUITests are new to this codebase, what are some of the resources you referenced?
We talked about this offline - that you had tried to add the accessibility identifier to the components in the demo controller rather than in the component itself, but it didn't work. I wonder if that's contributing to the increase in binary size? Maybe we can take another look at why setting the identifier in the demo controller component isn't working

And also great work on getting the first XCUITest up and running in FUA! Setting up anything new can be really daunting so great work diving in this :)

joannaquu · 2022-12-19T21:32:53Z

Closing to focus on launch tests first.

add initial activity indicator tests

7583450

joannaquu requested a review from a team as a code owner December 16, 2022 19:40

huwilkes reviewed Dec 16, 2022

View reviewed changes

ios/FluentUI/ActivityIndicator/ActivityIndicatorTokenSet.swift Outdated Show resolved Hide resolved

laminesm reviewed Dec 16, 2022

View reviewed changes

run linter + remove size extension

b5b3181

mischreiber reviewed Dec 16, 2022

View reviewed changes

joannaquu and others added 2 commits December 16, 2022 16:42

Merge branch 'main' into joannaqu/activity-indicator-test

c5570f6

remove FluentUI imports

85012ec

lyzhan7 reviewed Dec 19, 2022

View reviewed changes

joannaquu added 2 commits December 19, 2022 16:06

add constants for clarity

2c21384

update to work with different languages

465ff11

joannaquu closed this Dec 19, 2022

joannaquu deleted the joannaqu/activity-indicator-test branch December 27, 2022 16:26

joannaquu mentioned this pull request Jan 13, 2023

[XCUITests] Activity Indicator tests #1493

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XCUITests] Initial tests for Activity Indicator #1458

[XCUITests] Initial tests for Activity Indicator #1458

joannaquu commented Dec 16, 2022 •

edited

Loading

laminesm left a comment

mischreiber Dec 16, 2022

joannaquu Dec 16, 2022 •

edited

Loading

lyzhan7 Dec 19, 2022

lyzhan7 Dec 19, 2022

joannaquu Dec 19, 2022

lyzhan7 Dec 19, 2022

lyzhan7 Dec 19, 2022

lyzhan7 commented Dec 19, 2022

joannaquu commented Dec 19, 2022

[XCUITests] Initial tests for Activity Indicator #1458

[XCUITests] Initial tests for Activity Indicator #1458

Conversation

joannaquu commented Dec 16, 2022 • edited Loading

Platforms Impacted

Description of changes

Verification

Pull request checklist

Microsoft Reviewers: Open in CodeFlow

laminesm left a comment

Choose a reason for hiding this comment

mischreiber Dec 16, 2022

Choose a reason for hiding this comment

joannaquu Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

lyzhan7 Dec 19, 2022

Choose a reason for hiding this comment

lyzhan7 Dec 19, 2022

Choose a reason for hiding this comment

joannaquu Dec 19, 2022

Choose a reason for hiding this comment

lyzhan7 Dec 19, 2022

Choose a reason for hiding this comment

lyzhan7 Dec 19, 2022

Choose a reason for hiding this comment

lyzhan7 commented Dec 19, 2022

joannaquu commented Dec 19, 2022

joannaquu commented Dec 16, 2022 •

edited

Loading

joannaquu Dec 16, 2022 •

edited

Loading