search

misterspeedy / FsExcel

An F# Excel spreadsheet generator
MIT License
142 stars 18 forks source link

Make GitHub Actions run tests on Linux and Windows, adjust tests for Linux #52

Closed gdziadkiewicz closed 1 year ago

gdziadkiewicz commented 1 year ago

Why

@misterspeedy Thank you for creating this amazing library! FsExcel allowed me to quickly create readable code generating Excel sheets and avoid unwanted exposure to low-level libs.

The only problem was that my code base was on netstandard20. I locally resolved it by copying the code and compiling my netstandard20 version ( no code changes needed) but I felt that it would be nice to share this possibility with everyone.

I quickly found out that the current GitHub Actions build does not execute tests and that current tests are not Linux-ready. As I don't feel comfortable with proposing any changes (like the netstandard21 -> netstandard20 switch) without test results here I am with this PR that resolves issues blocking GitHub Action testing on Windows and Linux.

If I get this merged I will come back with the netstandard21 -> netstandard20 change trying to convince you that it is safe based on the tests that didn't get red from the change :)

How

  1. Changed the GitHub Actions build to work on the sln file rather than fsproj of the lib project. Additionally, added Windows build and the steps executed in vs code tasks to ensure that generated files are fresh and up to date.
  2. Added a workaround for the missing Calibri font preventing the opening of Excel files on Linux (https://docs.closedxml.io/en/latest/tips/missing-font.html)
  3. Resolved "/temp" path issues
  4. Ignored enumerate fonts test
gdziadkiewicz commented 1 year ago

(Build results on my fork can be found here https://github.com/gdziadkiewicz/FsExcel/pull/1 )

smoothdeveloper commented 1 year ago

@gdziadkiewicz I also want the library to target netstandard2.0 (to be useful on .NET Framework), and I faced similar issue for running the tests on macos.

I've made a PR which does only the test adjustments (#53) but without the need to remove the test with fonts, only skipping it if it is not running on windows for now.

I think rebasing this PR on top of #53 to only focus on CI and target would make sense (assuming the tests come green in your environment, without changing any .fs, .fsx, .xlsx files in this branch).

misterspeedy commented 1 year ago

WOW! Thank you so much @gdziadkiewicz and @smoothdeveloper ! Sorry for the delay but I have now merged the PRs. I didn't read your comment about rebasing until I had merged the main PR, but hopefully I have reinstated everything to what you both intended. The test pass on Windows for me.

Thank you so much for these improvements.

smoothdeveloper commented 1 year ago

@misterspeedy I'm confirming that the changes in my PR are indeed effective, able to run the tests and get those green.

If you could check #54 and publish the library again, I think it should enable the .net framework users to leverage the library.

Thanks for the stylish library!