Skip to content

propose edits to readme#4

Merged
noahsmartin merged 3 commits intogetsentry:mainfrom
armcknight:armcknight/proofread
May 29, 2025
Merged

propose edits to readme#4
noahsmartin merged 3 commits intogetsentry:mainfrom
armcknight:armcknight/proofread

Conversation

@armcknight
Copy link
Member

No description provided.

Copy link
Member Author

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

I generally tried to make this a bit less wordy, leaving some of the explanatory language for the blog post. This doc should be easy to scan for people to get up and running ASAP.

Comment on lines 7 to 9
1 - Use this library to generate an order file as part of your XCUITest. The library instruments app launch in the UI test and uses the results to generate an optimized order file
2 - Re-build the app with the order file. Once the order file is generated you build the app again, this time passing the order file as an option to the linker.

## Installation
1. Generate an order file as part of an XCUITest. FaultOrdering instruments app launch in the UI test and uses the results to generate an optimized order file.
2. Once the order file is generated, build the app again, this time passing the order file as an option to the linker.
Copy link
Member Author

Choose a reason for hiding this comment

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

Made these a markdown ordered list

Copy link
Collaborator

Choose a reason for hiding this comment

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

These things aren't part of the "installation" it's more of a general overview of what the process is, so I feel like it should still be a markdown list but above the "Package installation" header

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, they're still above the package installation header. After incorporating your feedback they're actually before ## Setup as well 👍🏻

README.md Outdated

Create a UI testing target using XCUITest. Add the package dependency to your Xcode project using the URL of this repository (https://github.com/getsentry/FaultOrdering).
Add `FaultOrderingTests` and `FaultOrdering` as a dependency of your new UI test target.
## Package Installation
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
## Package Installation
## Package installation

README.md Outdated
- Add `FaultOrderingTests` and `FaultOrdering` as dependencies of your new UI test target.

To use this package you'll need to tell Xcode to generate a linkmap for your main app binary. In your app's Xcode target, set the following build settings:
## Generating the linkmap
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is a different step than installation, so I promoted it to the same header level as ## Installation. Similar with most of the other headers.

After adding these settings, make sure to build your app and verify the file exists.

### Including the linkmap
> [!NOTE] We recommend using `$(PROJECT_DIR)` so that it generates within your project directory instead of derived data, but this can be changed to whatever makes sense for your setup.
Copy link
Member Author

Choose a reason for hiding this comment

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

Made this a note instead since it isn't technically required.

Once the `Linkmap.txt` file exists, it needs to be included as a resource in your UI test target. Add it in the build phases for your target under Copy Bundle Resources.
Add `Linkmap.txt` in the build phases for your UI test target under Copy Bundle Resources.

> [!IMPORTANT] Do **not** check the box to copy the file.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this "above the fold" as people might only see the step to add linkmap and skip over the screenshots if they think they already know how to do it, and this important bit was buried under the last one (i also still left it there)

Comment on lines -40 to +45
> [!IMPORTANT]
> The generated Linkmap.txt file must be included in your UI test target. You don't need to specificly use `"$(PROJECT_DIR)/Linkmap.txt"` as the path, only that whatever the `LD_MAP_FILE_PATH` is set is also the file that you include in Copy Bundle Resources.
> [!NOTE]
> You don't need to specifically use `"$(PROJECT_DIR)/Linkmap.txt"` as the path. Whatever value is set in `LD_MAP_FILE_PATH` is also the file that you include in Copy Bundle Resources.
Copy link
Member Author

Choose a reason for hiding this comment

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

This didn't actually seem super important, and we already call out in a similar note in its section that this isn't actually a requirement.

In a UI test, create an instance of `FaultOrderingTest` and optionally provide a closure to perform any necessary app setup. In most cases, this should include logging in to the app. Centering your UI test around a fully logged in session is strongly recommended, not only because it optimizes for the most common user experience, but also because it significantly improves the efficacy of this tool. By logging in, you allow much more of the app’s initial code to execute within the test context, allowing for a greater number of page fault reductions. The test case can then be executed.
In a UI test case, create an instance of `FaultOrderingTest` and optionally provide a closure to perform any necessary app setup.

> [!TIP] In most cases, this should include logging in to the app. Centering your UI test around a fully logged in session is strongly recommended, not only because it optimizes for the most common user experience, but also because it significantly improves the efficacy of this tool. By logging in, you allow much more of the app’s initial code to execute within the test context, allowing for a greater number of page fault reductions.
Copy link
Member Author

Choose a reason for hiding this comment

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

See discussion in slack about logging in.

let app = XCUIApplication()
let test = FaultOrderingTest { app in
// Perform setup such as logging in
import FaultOrderingTests
Copy link
Member Author

Choose a reason for hiding this comment

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

This didn't build until adding the import; i thought it'd be best to give a complete working snippet here.

> This test should run with the same compiler/linker optimizations that you would use for your App Store deliverable.

### Accessing results
### Device support
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved this section above "accessing results" since it's really a subdetail of how to run the ui test.

Once you run the UI test to generate an order file you have to use this file as an input to a new build of the app. Technically you only need to re-link the app, not re-compile everything, but running a new build with xcode is the easiest way to do this. Set the xcode build setting "ORDER_FILE" to the path to your order file when you build the app.
In your app target's build settings, set "ORDER_FILE" to the path to your order file and rebuild the app.

> [!TIP] Technically, the app only needs to be relinked to include the order file, as it has no effect on compilation of source code, but it may be easier to simply rebuild the entire app target.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is nice to know but i thought it better if it's separate.

@noahsmartin noahsmartin merged commit cb6b81c into getsentry:main May 29, 2025
2 checks passed
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.

2 participants