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

Introduce Windows CI, but fixed #268

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ElectronicRU
Copy link
Collaborator

Fixes bugs uncovered by #267.

List of changes:

  • Common utility file for initializing the state and discovering test app
  • Fix git flags in test_app_SUITE
  • Made output_dir test in sr_formatter_SUITE able to accept a path with a drive letter in it.

@ElectronicRU
Copy link
Collaborator Author

@paulo-ferraz-oliveira @elbrujohalcon could you take a look? CI fails with some stuff that's non-reproducible to me - it seems like it's improper stup/default, I'm not sure what to make of it.

Copy link
Collaborator

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

Nit-picking 🙈

@@ -0,0 +1,13 @@
-module(test_util).
Copy link
Collaborator

Choose a reason for hiding this comment

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

In NextRoll, we usually call modules like this one test_utils (plural).
Sorry about this… but… do you mind changing its name?

@elbrujohalcon
Copy link
Collaborator

elbrujohalcon commented Jun 11, 2021

Now, for that CI pipeline… No idea what's going on, but from what git diff is saying…

-        {you_cant, Open} when Open ->
+        throw:{you_cant, Open} when Open ->

The original file doesn't have that throw in there. Why would the formatter be adding it… and why only for Windows? 🤨

It's very strange to me that this happens only in the Windows version.
There must be something else that's different here.

@elbrujohalcon
Copy link
Collaborator

It might be the erl_syntax version…
It seems like, for Windows, the parser sees a class_qualifier in {you_cant, Open} when Open ->. While, in Linux, it parses it just as a tuple.
No idea how that happens.

@elbrujohalcon
Copy link
Collaborator

Also no idea what's the deal with a.broken.file… maybe it's adding (or subtracting) a newline at the end of it?

@ElectronicRU
Copy link
Collaborator Author

It might be the erl_syntax version…
It seems like, for Windows, the parser sees a class_qualifier in {you_cant, Open} when Open ->. While, in Linux, it parses it just as a tuple.
No idea how that happens.

I have to imagine that whatever vsn gets installed by gleam is wrong... the escript references "erl10.0" - is that right?

@elbrujohalcon
Copy link
Collaborator

I have to imagine that whatever vsn gets installed by gleam is wrong... the escript references "erl10.0" - is that right?

No idea. @paulo-ferraz-oliveira do you know?

@paulo-ferraz-oliveira
Copy link
Contributor

paulo-ferraz-oliveira commented Jun 14, 2021

I have to imagine that whatever vsn gets installed by gleam is wrong... the escript references "erl10.0" - is that right?

No idea. @paulo-ferraz-oliveira do you know?

Let me check (I think I've ran into this issue before).

@paulo-ferraz-oliveira
Copy link
Contributor

paulo-ferraz-oliveira commented Jun 14, 2021

So, https://github.com/gleam-lang/setup-erlang/blob/main/main.ps1#L4 seems to convert 21.3 to 21. We might just have to open an issue next to setup-erlang. Let me do that.

Edit: gleam-lang/setup-erlang#10

@ElectronicRU
Copy link
Collaborator Author

Would it be acceptable to bump gleam's version to 22 for now?

@paulo-ferraz-oliveira
Copy link
Contributor

I'm pretty sure 22 has no pre-compiled Windows versions (on the Erlang Solutions repo.), which is why I chose 21.3, but you can always try again, here, and we can BE sure 🤣 (I looked around and couldn't find a reference to where I state 22 isn't available - since I trust past-me).

@paulo-ferraz-oliveira
Copy link
Contributor

@ElectronicRU, any news on this? I'd like to test an update to the Windows CI, but would prefer to do it on an already working version 😄 Thanks.

@ElectronicRU
Copy link
Collaborator Author

@paulo-ferraz-oliveira Sorry, haven't been able to work on this - and honestly, I have a little clue as anyone at this point. :(

@paulo-ferraz-oliveira
Copy link
Contributor

Sure. I'll close my pull request, then, since this seems to be stalled. We can reopen later if required.

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