willowtreeapps / assertk

assertions for kotlin inspired by assertj
MIT License
760 stars 85 forks source link

Fix the build on Windows #482

Closed carltonwhitehead closed 1 year ago

carltonwhitehead commented 1 year ago

The project had been failing to build on my Windows PC with the following rather cryptic exception: Caused by: java.lang.StringIndexOutOfBoundsException: begin 324, end -1, length 5122

I tracked this down to use of hard-coded Unix-style line separator "\n" in TemplateTask.kt. Using the system's line separator corrected the problem.

I don't think this is likely to have any impact on non-Windows builds, as these templates are evidently dependent on the build platform, but releases already work on Windows, and clearly the build infrastructure is not Windows.

carltonwhitehead commented 1 year ago

I'm not sure anyone intended this project to build on Windows since the CI executors are "only" Mac and Linux. I had cloned it to a Windows machine to contribute back a few custom assertions, but couldn't get the project to build, so I went ahead and just tried to fix it. For a project without a Windows CI executor, it was a surprisingly easy fix.

It might be worth adding a Windows executor to the CircleCI config to help verify this change and protect against regressions. I'd be open to take care of that. Let me know if there is interest. Wanted to confirm before I spend more time on it.

fmedlin commented 1 year ago

Good catch! LGTM

evant commented 1 year ago

@carltonwhitehead Yeah, would have no problem adding a windows executor. Right now the macos executor builds the mac binaries and the linux executor builds the rest. (This is done to speed up build times since the mac executor is notably slower) So there's actually a couple of ways we could go with this.

  1. Just add the windows executor to test it builds but don't change anything else
  2. Move over the windows binary building to the windows executor

2 would be more 'consistent' but 1 would be a simpler change, leaning towards 1 but just thought I'd throw it out there