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

FreeBSD - tests/js/tconsole fails with different output #12182

Closed
euantorano opened this issue Sep 11, 2019 · 11 comments · Fixed by #14076
Closed

FreeBSD - tests/js/tconsole fails with different output #12182

euantorano opened this issue Sep 11, 2019 · 11 comments · Fixed by #14076

Comments

@euantorano
Copy link
Contributor

This is a bit of a weird one. On FreeBSD, tests/js/tconsole.nim fails with the following:

Category: js
Name: tests/js/tconsole.nim JS -d:nodejs
Action: run
Result: reOutputsDiffer
-------- Expected -------
Hello, console
1 2 3
1 'hi' 1.1
--------- Given  --------
Hello, console
1 2 3
1 hi 1.1
-------------------------

Category: js
Name: tests/js/tconsole.nim JS -d:nodejs -d:release
Action: run
Result: reOutputsDiffer
-------- Expected -------
Hello, console
1 2 3
1 'hi' 1.1
--------- Given  --------
Hello, console
1 2 3
1 hi 1.1
-------------------------

Node is installed via pkg install node and reports the following version information: v12.4.0

@euantorano
Copy link
Contributor Author

Just to test it out, I created a file index.js:

console.log("Hello, console")
console.log(1, 2, 3)
console.log(1, "hi", 1.1)

And ran it with node index.js:

Hello, console
1 2 3
1 hi 1.1

I did the same on the Mac (Node v12.10.0) and got the exact same output, but the test doesn't seem to fail on the Mac CI...

@Araq
Copy link
Member

Araq commented Sep 12, 2019

For me it produces the single quotes indeed, I'm on node --version v6.10.0

@euantorano
Copy link
Contributor Author

euantorano commented Sep 12, 2019 via email

@euantorano
Copy link
Contributor Author

I've just tested Node v10.16.0 via https://repl.it/languages/nodejs and it outputs:

Hello, console
1 2 3
1 'hi' 1.1

It definitely looks like Node have changed something from v10 - v12 😢

I'm not sure what to do with these tests in that case, but I'm going to contact somebody I know who's more involved with Node to see if they have any ideas when it changed.

@Araq
Copy link
Member

Araq commented Sep 12, 2019

Just disable the test for OpenBSD, it's not that important...

@euantorano
Copy link
Contributor Author

@Araq Yep, we can do that for FreeBSD.

However, there may be a case in the future where these tests suddenly start failing elsewhere in CI due to Node updates. Just something that we should be wary of.

@euantorano
Copy link
Contributor Author

Looks like the change happened in Node v12. Thanks to @bnb for opening an issue in the Node repository and helping track this down.

The AppVeyor script currently installs Node 8.

The Travis script doesn't install Node specifically, as Node is installed as part of the build environment. Currently, the xenial build environment installs both v11.0.0 and v8.12.0.

I've ignored the test on FreeBSD now, so this issue can be closed when the FreeBSD CI PR gets merged.

@stefantalpalaru
Copy link
Contributor

I see the same test failures on Linux with Node.js 12.10.0.

@euantorano
Copy link
Contributor Author

euantorano commented Sep 24, 2019 via email

euantorano added a commit to euantorano/Nim that referenced this issue Nov 11, 2019
euantorano added a commit to euantorano/Nim that referenced this issue Nov 29, 2019
@Araq Araq closed this as completed in c5c6bae Nov 29, 2019
@stefantalpalaru
Copy link
Contributor

Why would you disable a test on FreeBSD when the output mismatch is also seen on Linux and it's due to a newer Node.js version?

@euantorano
Copy link
Contributor Author

euantorano commented Nov 30, 2019 via email

alehander92 pushed a commit to alehander92/Nim that referenced this issue Dec 2, 2019
* Ref nim-lang#12103 - adds FreeBSD CI
* Fix getApplFreebsd - length of the string includes the null terminator byte, so minus 1 for result length
* Show last commit in setup task.
* Remove .git from repository URL
* Don't include noisy details showing last commit.
* Add FreeBSD build status badge
* Fix nim-lang#12182 - disable tconsole on FreeBSD
* Disable tgetaddrinfo on FreebSD as getaddrinfo doesn't support the ICMP protocol.
* Install boehm-gc-threaded
* Use libgc-threaded.so on FreeBSD rather than libgc.so.
* Simplify build failure handling. Update alt text for CI badge.
* Disable test on FreeBSD
* Simplify build config

- use GNU make to build csources
- set PATH variable using the environment key
- remove modification of config to set CC as this is already set

* Install git which seems to be missing from current freebsd images
* Revert change to how path is set
* Add a comment explaining why the length is truncated
* Fix tconsole.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants