Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Xcode Runtime issue breakpoints #761

Merged

Conversation

zenangst
Copy link
Contributor

@zenangst zenangst commented Jun 16, 2023

Short description 📝

This PR adds support for Xcode runtime issues breakpoints.
It simply adds a new case to the BreakpointExtensionID called .runtimeException which can be used to generate breakpoints.

Solution 📦

Add missing case to BreakpointExtensionID for generating runtime issue breakpoints.

Implementation 👩‍💻👨‍💻

  • Adds new BreakpointExtensionID case to support runtime issue breakpoints
  • Expands the XCBreakpointListTests to include looking for the runtime exception
  • Update the breakpoints fixture to include a runtime issue breakpoint

- Adds new `BreakpointExtensionID` case to support runtime issue breakpoints
- Expands the `XCBreakpointListTests` to include looking for the runtime exception
- Update the breakpoints fixture to include a runtime issue breakpoint
@kwridan
Copy link
Collaborator

kwridan commented Jun 19, 2023

@all-contributors add @zenangst for code

@allcontributors
Copy link
Contributor

@kwridan

I've put up a pull request to add @zenangst! 🎉

@zenangst
Copy link
Contributor Author

Found a nasty little casing issue which caused the tests to not compile. Fixed here ad3d477

@zenangst
Copy link
Contributor Author

I bit unsure what is going on with the test_read_write_produces_no_diff test.

- Ensure fixture is identical to the way XcodeProj serialises it
- This allows diff tests to verify the write process is sable
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks @zenangst

This is a good addition 👍 previously any breakpoints list with runtime exceptions within it would completely fail to parse.

One small suggestion / improvement for completeness, runtime exceptions have an associated type that is encoded within BreakpointContent for example:

<BreakpointProxy
   BreakpointExtensionID = "Xcode.Breakpoint.RuntimeIssueBreakpoint">
   <BreakpointContent
      uuid = "0E9F84AF-42FF-4236-B75A-97E449E6C66F"
      shouldBeEnabled = "Yes"
      ignoreCount = "0"
      continueAfterRunningActions = "No"
      breakpointStackSelectionBehavior = "1"
      type = "1">
   </BreakpointContent>
</BreakpointProxy>

From a quick trial, I've found the following:

  • Thread Sanitizer: type = 1
  • Undefined Behaviour: type = 2
  • Main thread checker: type = 4
  • System Frameworks: type = 8
  • All - type = 65535

It appears to be a 16bit mask. It can be modelled as an option set. That said seeing it's only applicable to runtime exceptions it would mean it would be unused / optional for other types. I haven't quite figured out what a good public API for it would be. Perhaps can be done separately with some more thought put into it - mind raising an issue for it and if you have any ideas / suggestions?

Thanks again!

@zenangst
Copy link
Contributor Author

@kwridan I tried something really quickly but I ended up with the clash between using CaseIterable and having an associated type on the enum. They don't really work well together.

I think to really nails this, I'll open an issue, so that the rest of the community can also chip in an spitball some ideas, like you suggested.

I pushed a commit to improve on the naming.

@zenangst
Copy link
Contributor Author

Made an issue here - #763

@kwridan kwridan requested a review from brentleyjones June 20, 2023 07:21
@kwridan
Copy link
Collaborator

kwridan commented Jun 20, 2023

Awesome thanks @zenangst 🙏

@pepicrft
Copy link
Contributor

Thanks for tackling this one @zenangst. I'll go ahead with merging this and I'll tackle your issue separately.

@pepicrft pepicrft merged commit 6097424 into tuist:main Jun 26, 2023
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.

3 participants