Fix CI tests for macoOS and ubuntu OS. #538
Conversation
| @@ -19,7 +19,7 @@ jobs: | |||
| version: | |||
| - '1.10' # Replace this with the minimum Julia version that your package supports. E.g. if your package requires Julia 1.5 or higher, change this to '1.5'. | |||
| - '1' # Leave this line unchanged. '1' will automatically expand to the latest stable 1.x release of Julia. | |||
| os: [ubuntu-latest, windows-latest, macos-latest] | |||
There was a problem hiding this comment.
Why Is macos-latest not working? Is it because of Kaileido_jll? Check the PR I created about that.
There was a problem hiding this comment.
I tested removing the package in e208255 , but that wasn't the reason. I still don't know why it doesn't work in the latest version. I'll keep it in macOS-13 until it's solved.
test/runtests.jl
Outdated
| # Opens UI | ||
| w = KomaUI(return_window=true) | ||
|
|
||
| @unsafe_blink |
There was a problem hiding this comment.
Yo actually fix the bug (opposed to only fix the tests) the call to the macro needs to be inside the KomaUI function.
|
@Stockless can you reply to the comments 🙏 ? |
|
please don't include changes from other PRs without merging them properly, so the ownership of the changes is not changed. |
|
Hi @Stockless, What is the state of this? |
|
This branch is already outdated. Please don't add new dependencies unless strictly necessary. |
| @@ -20,7 +20,7 @@ KomaPlotsPlutoPlotlyExt = "PlutoPlotly" | |||
|
|
|||
| [compat] | |||
| Interpolations = "0.13, 0.14, 0.15" | |||
| Kaleido_jll = "0.1" | |||
There was a problem hiding this comment.
Why are you putting this here? The solution should be to remove the compat. This is already done in #530
There was a problem hiding this comment.
I did remove Kaleido_jll to test if that was the reason why the CI test in macOS-latest wasn't running, but when you told me that this change shouldn't be included because it is from another PR, I did an stepback in this commit c0ce80e
There was a problem hiding this comment.
Yes, but now it says Kaleido_jll = "0.2.1" which is not the same as before Kaleido_jll = "0.1"
| @@ -14,6 +14,7 @@ KomaMRIPlots = "76db0263-63f3-4d26-bb9a-5dba378db904" | |||
| MAT = "23992714-dd62-5051-b70f-ba57cb901cac" | |||
| MRIReco = "bdf86e05-2d2b-5731-a332-f3fe1f9e047f" | |||
| Reexport = "189a3867-3050-52da-a836-e630ba90ab69" | |||
| Sockets = "6462fe0b-24de-5631-8697-dd941f90decc" | |||
There was a problem hiding this comment.
I'm adding Sockets because it is part of the implementation of a macro to disable the sandbox in the window generation, creating a workaround for the connection bug as it is described in this closed issue
There was a problem hiding this comment.
Sockets should already be included in Blink. Use why Sockets to check. For example, you could do using Blink.HTTP.Sockets. Sockets shouldn't be an explicit dependency in KomaMRI.
| using Blink, Interact, AssetRegistry, Blink.AtomShell | ||
| using Sockets: @ip_str | ||
| using Blink.AtomShell: port, inspector, try_connect, electron, initcbs, mainjs |
There was a problem hiding this comment.
| using Blink, Interact, AssetRegistry, Blink.AtomShell | |
| using Sockets: @ip_str | |
| using Blink.AtomShell: port, inspector, try_connect, electron, initcbs, mainjs | |
| using Blink, Interact, AssetRegistry |
| @@ -0,0 +1,34 @@ | |||
| module MacroUnsafeBlink | |||
There was a problem hiding this comment.
I'm currently working in the implementation of the macro that fixed CI test in commit c5117f0 inside the KomaUI function, and it's easier for me to run the different test from a module and import it. In a later version it should be incorporated directly in the function instead of the module import.
There was a problem hiding this comment.
OK, please read carefully this https://jkrumbiegel.com/pages/2021-06-07-macros-for-beginners/ to understand better how macros work.
Fixed the CI test for macOS replacing the OS version from macOS-latest to macOS-13, and the test for Ubuntu adding a macro to avoid the generation of a sandbox in the creation of the gui window, that is causing an issue due to a bug in the latest version of Ubuntu (Ubuntu 24.04.1)