xcresult icon indicating copy to clipboard operation
xcresult copied to clipboard

Generate models automatically to keep them up-to-date

Open ainame opened this issue 3 years ago • 4 comments

Prior to this PR, xcresult gem has been using (semi?) handcrafted models to parse .xcresult data, I assume. So that it's not up-to-date to the latest version nor implementing all of the properties on each type.

This PR aims to switch the strategy to maintain xcresult to using a script to generate models based on what xcrun xcresulttool formatDescription get --format json outputs.

As a result, we've got new models related to ActivityLog (honestly I don't know what it is though😅) and most importantly it's got "version" info and the timestamp of the last update in its code.

With this PR, the file structure related to models will be like below. (Naming suggestions are very welcome🙏)

  • lib/xcresult/models.gen.rb - automatically generated file that defines models and is supposed to be updated by bin/generate_models script
  • lib/xcresult/models.rb - entrypoint to do require models.gen and add convenient methods to defined models

bin/generate_models script is the one we can use to update lib/xcresult/models.gen.rb. We might be able to automate to create PRs to keep models up-to-date in future but for now, we need to run it manually.

One thing I need to mention is that this time I wrapped all models into another nested namespace XCResult::Models to make it easy to differentiate other classes that exist other than representing models. And you won't need to worry about the conflict of naming with what Apple gives us in the future. I started using dynamic class loading like Kernel.const_get("XCResult::Models::#{type_name}"), which makes implementing polymorphic objects easier a lot. If we don't use a dedicated namespace, it's less likely but may happen to load the wrong classes. It's safer to assume classes under XCResult::Models namespace are for models parsing the JSON.

This is a breaking change but I would justify it for the following reasons, in my opinion.

  1. This gem hasn't reached v1.0 so semantically it can include breaking changes anytime according to semantic versioning rules
  2. In reality, most users don't care what classes this gem internally uses with usages like in README; i.e. parser.export_xccovreports(destination: './outputs') https://github.com/fastlane-community/xcov/blob/f8904c7f000b415c7800b1c68c50e8de9dab76da/lib/xcov/manager.rb#L217-L221
  3. Even if it breaks something, required migration is super easy (appending ::Models) and only once but the benefit will be forever

I can easily remove the additional nest of the namespace (::Models) if you are concerned about compatibility. Please let me know🙂


JFYI: After merging this PR, I'd like to add screenshot extraction functionality to this gem 🏃

ainame avatar Jul 20 '21 22:07 ainame