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

Tests fail, expecting more libs to be loaded #94

Closed
rocketnia opened this Issue Feb 12, 2018 · 17 comments

Comments

Projects
None yet
3 participants
@rocketnia
Member

rocketnia commented Feb 12, 2018

The CI tests are currently failing. A quick fix is to uncomment all the libraries listed in libs.arc.

@hjek, was there a reason you commented these out a while ago? And is there some sort of rubric you're using for calling them "optional"? I would uncomment them myself, but it looks like you have something in mind with what you're doing, and I might trample on that.

@hjek

This comment has been minimized.

Show comment
Hide comment
@hjek

hjek Feb 13, 2018

Contributor

Oops. I'll look into this.

I've been going through some of the many libraries that come in the repo, and some currently work and are quite interesting (in particular the blog.arc and prompt.arc) whereas others currently need some work to actually load.

The libraries that are commented out in libs.arc are libraries that are already working, but not strictly required to load news.arc. Loading many libraries can make Arc a bit slower, and I've been trying to get the repl to boot faster, as it was really slow.

I'd guess the issue here is that I've accidentally commented out one library that is not strictly required by News, yet still needed for the unit tests to pass.

I'm fine looking into it soon, but of course it's fine if you can look at it / fix it (and I don't see that as trampling).

Contributor

hjek commented Feb 13, 2018

Oops. I'll look into this.

I've been going through some of the many libraries that come in the repo, and some currently work and are quite interesting (in particular the blog.arc and prompt.arc) whereas others currently need some work to actually load.

The libraries that are commented out in libs.arc are libraries that are already working, but not strictly required to load news.arc. Loading many libraries can make Arc a bit slower, and I've been trying to get the repl to boot faster, as it was really slow.

I'd guess the issue here is that I've accidentally commented out one library that is not strictly required by News, yet still needed for the unit tests to pass.

I'm fine looking into it soon, but of course it's fine if you can look at it / fix it (and I don't see that as trampling).

@akkartik

This comment has been minimized.

Show comment
Hide comment
@akkartik

akkartik Feb 13, 2018

Member

I suggest we split off a new list of files to load so that CI always runs all available tests.

Member

akkartik commented Feb 13, 2018

I suggest we split off a new list of files to load so that CI always runs all available tests.

@rocketnia rocketnia closed this in 63de938 Feb 16, 2018

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 16, 2018

Member

The way I fixed this was to have most of the tests load the libraries they depend on, with one significant exception:

I uncommented lib/make-br-fn.arc and moved to the "core" list. I did this because it's one of the incompatible changes in Anarki that's documented in the CHANGES/ directory, so for it not to be active all the time would be a little odd.

Unfortunately, I suspect this file could contribute somewhat to Anarki's startup time, since it changes the [...] square bracket syntax into a macro that code-walks over its body and speculatively ssexpands everything to find post-ssexpansion occurrences of symbols whose names begin with _. In my other language projects, I've always found string processing to be where all my startup time goes.

Then again, Arc already does ssexpansion for all code, so whatever this adds might not be that bad. Arc loads pretty fast for me either way, so I'm not sure.

@hjek, if you notice this causes a substantial regression in load times, I think we have lots of options. Not too much really depends on make-br-fn.arc (just a few things in util.arc, ppr.arc, and client.arc.). We can probably refactor those files, the way make-br-fn.arc works, and the CHANGES/ documentation so it isn't running an expensive string-processing operation for every occurrence of [...].

Member

rocketnia commented Feb 16, 2018

The way I fixed this was to have most of the tests load the libraries they depend on, with one significant exception:

I uncommented lib/make-br-fn.arc and moved to the "core" list. I did this because it's one of the incompatible changes in Anarki that's documented in the CHANGES/ directory, so for it not to be active all the time would be a little odd.

Unfortunately, I suspect this file could contribute somewhat to Anarki's startup time, since it changes the [...] square bracket syntax into a macro that code-walks over its body and speculatively ssexpands everything to find post-ssexpansion occurrences of symbols whose names begin with _. In my other language projects, I've always found string processing to be where all my startup time goes.

Then again, Arc already does ssexpansion for all code, so whatever this adds might not be that bad. Arc loads pretty fast for me either way, so I'm not sure.

@hjek, if you notice this causes a substantial regression in load times, I think we have lots of options. Not too much really depends on make-br-fn.arc (just a few things in util.arc, ppr.arc, and client.arc.). We can probably refactor those files, the way make-br-fn.arc works, and the CHANGES/ documentation so it isn't running an expensive string-processing operation for every occurrence of [...].

@rocketnia rocketnia reopened this Feb 16, 2018

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 16, 2018

Member

I've reopened this because even though my local ./arc.sh -n tests.arc command is working, Travis still seems to be failing for other reasons.

Member

rocketnia commented Feb 16, 2018

I've reopened this because even though my local ./arc.sh -n tests.arc command is working, Travis still seems to be failing for other reasons.

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 16, 2018

Member

Looks like commit d869498 fixed it.

Member

rocketnia commented Feb 16, 2018

Looks like commit d869498 fixed it.

@hjek

This comment has been minimized.

Show comment
Hide comment
@hjek

hjek Feb 16, 2018

Contributor

@rocketnia Cool. make-br-fn.arc should obviously be loaded, if it's necessary for Arc to work properly, regardless of hypothetical slowdown. It's just me who hadn't been careful enough when commenting all those libs out.

Contributor

hjek commented Feb 16, 2018

@rocketnia Cool. make-br-fn.arc should obviously be loaded, if it's necessary for Arc to work properly, regardless of hypothetical slowdown. It's just me who hadn't been careful enough when commenting all those libs out.

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 17, 2018

Member

I don't exactly want to say make-br-fn.arc is sacred. I'm just explaining why I reintroduced it: Because it would take a few changes across several files to properly remove it (or alternatively to move it to its own non-conflicting library). If reintroducing it is fine for your startup time purposes, then that sounds good to me.

...

Before I close this, I also want to elaborate on what I had missed the first commit that required a second one. Anarki is a Racket package (and I should know, because I made it so :-p ). As such, it a certain process it goes through when installing the package or running the Racket's unit test runner.

In the Travis CI script, I already have it running raco commands to put those things through their paces. Here are the commands, along with some on-the-spot descriptions of what they do (not quite an excerpt of .travis.yaml):

# Run the Arc tests.
./arc.sh -n tests.arc

# Install the current directory as a package in the Racket
# installation, as well as its dependencies.
raco pkg install --deps search-auto

# Recompile the `anarki` package and make sure it has no undeclared
# dependencies.
raco setup --no-docs --check-pkg-deps anarki

# Run tests on all the *.rkt files in the `anarki` package.
raco test --no-run-if-absent --package anarki

So I ran the raco pkg install --deps search-auto command locally... and somehow sorta corrupted my Racket installation. I already knew there were weird file permission inconsistencies in my Racket installation directory, but this was finally the last straw, and Arc gave me a strange error when I tried to run it after that. I don't have the details handy, and I don't think this problem will affect most people.

Once I deleted and reinstalled Racket, I ran these commands and it all worked out fine.

...

Hm. It looks like there's one more place the CI tests are failing. This time it's not Travis CI, but by the CI testing at the Racket package catalog:

The time is now Friday, February 16th, 2018 6:09:03am
/usr/bin/ssh -R 18333:localhost:18333 racket@192.168.56.107 '/usr/bin/env' 'DISPLAY=:1' 'PLT_PKG_BUILD_SERVICE=1' 'PLTUSERHOME=/home/racket/build-pkgs/user' 'PLT_PKG_BUILD_SERVICE=1' 'CI=true' 'PLT_INFO_ALLOW_VARS=;PLT_PKG_BUILD_SERVICE' '/bin/sh' '-c' 'cd "/home/racket/build-pkgs"/racket && bin/raco pkg install -u --auto anarki && bin/raco test --drdr --package anarki'
Resolving "anarki" via http://localhost:18333/built/catalog/
Downloading http://localhost:18333/built/pkgs/anarki.zip
raco setup: version: 6.12
raco setup: platform: x86_64-linux-natipkg [3m]
raco setup: installation name: 6.12
raco setup: variants: 3m
raco setup: main collects: /home/racket/build-pkgs/racket/collects
raco setup: collects paths: 
raco setup:   /home/racket/build-pkgs/user/.racket/6.12/collects
raco setup:   /home/racket/build-pkgs/racket/collects
raco setup: main pkgs: /home/racket/build-pkgs/racket/share/pkgs
raco setup: pkgs paths: 
raco setup:   /home/racket/build-pkgs/racket/share/pkgs
raco setup:   /home/racket/build-pkgs/user/.racket/6.12/pkgs
raco setup: links files: 
raco setup:   /home/racket/build-pkgs/racket/share/links.rktd
raco setup:   /home/racket/build-pkgs/user/.racket/6.12/links.rktd
raco setup: main docs: /home/racket/build-pkgs/racket/doc
raco setup: --- updating info-domain tables ---
raco setup: updating: /home/racket/build-pkgs/user/.racket/6.12/share/info-cache.rktd
raco setup: --- pre-installing collections ---
raco setup: --- installing foreign libraries ---
raco setup: --- installing shared files ---
raco setup: --- compiling collections ---
raco setup: making: <pkgs>/anarki
raco setup: making: <pkgs>/anarki/CHANGES
raco setup: making: <pkgs>/anarki/extras
raco setup: making: <pkgs>/anarki/extras/vim
raco setup: making: <pkgs>/anarki/extras/vim/ftdetect
raco setup: making: <pkgs>/anarki/extras/vim/ftplugin
raco setup: making: <pkgs>/anarki/extras/vim/indent
raco setup: making: <pkgs>/anarki/extras/vim/syntax
raco setup: making: <pkgs>/anarki/lang
raco setup: making: <pkgs>/anarki/lib
raco setup: making: <pkgs>/anarki/lib/racket-lang-demo
raco setup: making: <pkgs>/anarki/lib/tests
raco setup: making: <pkgs>/anarki/static
raco setup: --- creating launchers ---
raco setup: --- installing man pages ---
raco setup: --- building documentation ---
raco setup: --- installing collections ---
raco setup: --- post-installing collections ---
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/ac.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/boot.rkt"
boot.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/brackets.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/info.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/module-begin.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/reader.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/json.ss"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-application.rkt"
lang-anarki-application.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-deep-dependency.rkt"
lang-anarki-deep-dependency.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-library.rkt"
lang-anarki-library.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-application.rkt"
racket-application.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-deep-dependency.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/main.rkt"
5/5 test failures
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/ac.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/brackets.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/info.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/module-begin.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/reader.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/json.ss
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-deep-dependency.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/main.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/boot.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-application.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-deep-dependency.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-library.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-application.rkt
The time is now Friday, February 16th, 2018 6:09:30am

It looks like the tests are still failing because some output is sent to stderr when one of the *.rkt modules is run. It looks like there's something of a dicrepancy between the testing command raco test --no-run-if-absent --package anarki that I got from travis-racket and the command raco test --drdr --package anarki that the Racket package catalog uses. We probably oughta use something more like the latter in our Travis CI script so that we don't have to wait multiple hours at a time for the Racket package catalog to run another round of tests.

I'll see what I can do to make this work.

I feel like the "initializing arc" message should show when running Anarki at the command line, but that it shouldn't show when running Anarki programmatically through the anarki module in Racket until someday when there's a "verbose" flag the programmer can provide. So to get the tests to work, I'll likely just change what part of the code displays that message.

Member

rocketnia commented Feb 17, 2018

I don't exactly want to say make-br-fn.arc is sacred. I'm just explaining why I reintroduced it: Because it would take a few changes across several files to properly remove it (or alternatively to move it to its own non-conflicting library). If reintroducing it is fine for your startup time purposes, then that sounds good to me.

...

Before I close this, I also want to elaborate on what I had missed the first commit that required a second one. Anarki is a Racket package (and I should know, because I made it so :-p ). As such, it a certain process it goes through when installing the package or running the Racket's unit test runner.

In the Travis CI script, I already have it running raco commands to put those things through their paces. Here are the commands, along with some on-the-spot descriptions of what they do (not quite an excerpt of .travis.yaml):

# Run the Arc tests.
./arc.sh -n tests.arc

# Install the current directory as a package in the Racket
# installation, as well as its dependencies.
raco pkg install --deps search-auto

# Recompile the `anarki` package and make sure it has no undeclared
# dependencies.
raco setup --no-docs --check-pkg-deps anarki

# Run tests on all the *.rkt files in the `anarki` package.
raco test --no-run-if-absent --package anarki

So I ran the raco pkg install --deps search-auto command locally... and somehow sorta corrupted my Racket installation. I already knew there were weird file permission inconsistencies in my Racket installation directory, but this was finally the last straw, and Arc gave me a strange error when I tried to run it after that. I don't have the details handy, and I don't think this problem will affect most people.

Once I deleted and reinstalled Racket, I ran these commands and it all worked out fine.

...

Hm. It looks like there's one more place the CI tests are failing. This time it's not Travis CI, but by the CI testing at the Racket package catalog:

The time is now Friday, February 16th, 2018 6:09:03am
/usr/bin/ssh -R 18333:localhost:18333 racket@192.168.56.107 '/usr/bin/env' 'DISPLAY=:1' 'PLT_PKG_BUILD_SERVICE=1' 'PLTUSERHOME=/home/racket/build-pkgs/user' 'PLT_PKG_BUILD_SERVICE=1' 'CI=true' 'PLT_INFO_ALLOW_VARS=;PLT_PKG_BUILD_SERVICE' '/bin/sh' '-c' 'cd "/home/racket/build-pkgs"/racket && bin/raco pkg install -u --auto anarki && bin/raco test --drdr --package anarki'
Resolving "anarki" via http://localhost:18333/built/catalog/
Downloading http://localhost:18333/built/pkgs/anarki.zip
raco setup: version: 6.12
raco setup: platform: x86_64-linux-natipkg [3m]
raco setup: installation name: 6.12
raco setup: variants: 3m
raco setup: main collects: /home/racket/build-pkgs/racket/collects
raco setup: collects paths: 
raco setup:   /home/racket/build-pkgs/user/.racket/6.12/collects
raco setup:   /home/racket/build-pkgs/racket/collects
raco setup: main pkgs: /home/racket/build-pkgs/racket/share/pkgs
raco setup: pkgs paths: 
raco setup:   /home/racket/build-pkgs/racket/share/pkgs
raco setup:   /home/racket/build-pkgs/user/.racket/6.12/pkgs
raco setup: links files: 
raco setup:   /home/racket/build-pkgs/racket/share/links.rktd
raco setup:   /home/racket/build-pkgs/user/.racket/6.12/links.rktd
raco setup: main docs: /home/racket/build-pkgs/racket/doc
raco setup: --- updating info-domain tables ---
raco setup: updating: /home/racket/build-pkgs/user/.racket/6.12/share/info-cache.rktd
raco setup: --- pre-installing collections ---
raco setup: --- installing foreign libraries ---
raco setup: --- installing shared files ---
raco setup: --- compiling collections ---
raco setup: making: <pkgs>/anarki
raco setup: making: <pkgs>/anarki/CHANGES
raco setup: making: <pkgs>/anarki/extras
raco setup: making: <pkgs>/anarki/extras/vim
raco setup: making: <pkgs>/anarki/extras/vim/ftdetect
raco setup: making: <pkgs>/anarki/extras/vim/ftplugin
raco setup: making: <pkgs>/anarki/extras/vim/indent
raco setup: making: <pkgs>/anarki/extras/vim/syntax
raco setup: making: <pkgs>/anarki/lang
raco setup: making: <pkgs>/anarki/lib
raco setup: making: <pkgs>/anarki/lib/racket-lang-demo
raco setup: making: <pkgs>/anarki/lib/tests
raco setup: making: <pkgs>/anarki/static
raco setup: --- creating launchers ---
raco setup: --- installing man pages ---
raco setup: --- building documentation ---
raco setup: --- installing collections ---
raco setup: --- post-installing collections ---
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/ac.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/boot.rkt"
boot.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/brackets.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/info.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/module-begin.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/reader.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/json.ss"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-application.rkt"
lang-anarki-application.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-deep-dependency.rkt"
lang-anarki-deep-dependency.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-library.rkt"
lang-anarki-library.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-application.rkt"
racket-application.rkt: raco test: non-empty stderr: #"initializing arc.. (may take a minute)\n"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-deep-dependency.rkt"
raco test: "/home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/main.rkt"
5/5 test failures
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/ac.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/brackets.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/info.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/module-begin.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lang/reader.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/json.ss
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-deep-dependency.rkt
  0 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/main.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/boot.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-application.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-deep-dependency.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/lang-anarki-library.rkt
1 1 /home/racket/build-pkgs/user/.racket/6.12/pkgs/anarki/lib/racket-lang-demo/racket-application.rkt
The time is now Friday, February 16th, 2018 6:09:30am

It looks like the tests are still failing because some output is sent to stderr when one of the *.rkt modules is run. It looks like there's something of a dicrepancy between the testing command raco test --no-run-if-absent --package anarki that I got from travis-racket and the command raco test --drdr --package anarki that the Racket package catalog uses. We probably oughta use something more like the latter in our Travis CI script so that we don't have to wait multiple hours at a time for the Racket package catalog to run another round of tests.

I'll see what I can do to make this work.

I feel like the "initializing arc" message should show when running Anarki at the command line, but that it shouldn't show when running Anarki programmatically through the anarki module in Racket until someday when there's a "verbose" flag the programmer can provide. So to get the tests to work, I'll likely just change what part of the code displays that message.

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 17, 2018

Member

Before I close this, [...]

Hm. It looks like there's one more place the CI tests are failing.

Heh, you can see I was about to close this, but I don't think it's quite time yet. 😜

Member

rocketnia commented Feb 17, 2018

Before I close this, [...]

Hm. It looks like there's one more place the CI tests are failing.

Heh, you can see I was about to close this, but I don't think it's quite time yet. 😜

@rocketnia rocketnia self-assigned this Feb 17, 2018

rocketnia added a commit that referenced this issue Feb 17, 2018

Make the `raco test` command in .travis.yml use the `--drdr` option i…
…nstead of the `--no-run-if-absent` option, bringing it closer to the way the tests are run at the Racket package index. Relates to #94.

@rocketnia rocketnia closed this in f8170df Feb 17, 2018

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 17, 2018

Member

Nope, apparently not fixed yet.

Member

rocketnia commented Feb 17, 2018

Nope, apparently not fixed yet.

@rocketnia rocketnia reopened this Feb 17, 2018

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 17, 2018

Member
lang-anarki-deep-dependency.rkt: raco test: non-empty stderr: #"define-values: assignment disallowed;\n cannot re-define a constant\n  constant: lifted.0\n  in module: \"/home/travis/build/arclanguage/anarki/ac.rkt\"\n  context...:\n   /home/travis/build/arclanguage/anarki/ac.rkt:1295:0: aload1\n   /home/travis/build/arclanguage/anarki/boot.rkt:18:2: anarki-init-promise\n   /home/travis/racket/collects/racket/private/promise.rkt:96:0: force/generic\n   /home/travis/build/arclanguage/anarki/lib/racket-lang-demo/lang-anarki-deep-dependency.rkt: [running body]\n   (submod /home/travis/racket/share/pkgs/compiler-lib/compiler/commands/test.rkt process): [running body]\n"

The "cannot re-define a constant\n constant: lifted.0" error message is the same perplexing error message I was getting before I decided to reinstall Racket.

Member

rocketnia commented Feb 17, 2018

lang-anarki-deep-dependency.rkt: raco test: non-empty stderr: #"define-values: assignment disallowed;\n cannot re-define a constant\n  constant: lifted.0\n  in module: \"/home/travis/build/arclanguage/anarki/ac.rkt\"\n  context...:\n   /home/travis/build/arclanguage/anarki/ac.rkt:1295:0: aload1\n   /home/travis/build/arclanguage/anarki/boot.rkt:18:2: anarki-init-promise\n   /home/travis/racket/collects/racket/private/promise.rkt:96:0: force/generic\n   /home/travis/build/arclanguage/anarki/lib/racket-lang-demo/lang-anarki-deep-dependency.rkt: [running body]\n   (submod /home/travis/racket/share/pkgs/compiler-lib/compiler/commands/test.rkt process): [running body]\n"

The "cannot re-define a constant\n constant: lifted.0" error message is the same perplexing error message I was getting before I decided to reinstall Racket.

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 17, 2018

Member

The error seems to be related to a bug in Racket that was introduced in version 6.3. I tried on several of the latest versions of Racket on Linux, and the most recent versions seem to work:

  • 6.8: Fails. (This is the version our Travis CI tests currently use.)
  • 6.9: Fails.
  • 6.10: Fails.
  • 6.10.1: Fails.
  • 6.11: Works.
  • 6.12: Works. (This is the version the Racket package index currently uses.)

Looking through Racket's commit history, I notice that indeed there's a fix for this just before version 6.11. (Hey, there's also a fix for text input streams on Windows! I hope that fixes the pasting-at-the-REPL issue.)

I tested with various old versions of Anarki too. It looks like this error started happening during my contributions last year, beginning with commit 2946803, which coincidentally(?) is the first commit where the Travis CI tests first started to work. This "lifted.0" error does sound distantly familiar... Maybe I saw it failing on the Racket package index at one point and just put it out of mind because I had no idea what to do about it.

I tinkered a little bit with that commit, and removing ac.scm's new dependency on racket/random seems to fix it. Back to the present-day state of the codebase, removing ac.rkt's dependencies on racket/random and openssl seems to fix it too. It's not as if we have to replace the functionality somehow, 'cause it seems to work if I replace ssl-connect with (dynamic-require 'openssl 'ssl-connect) and crypto-random-bytes with (dynamic-require 'racket/random 'crypto-random-bytes).

So it seems we have two pretty reasonable options here:

  • Change .travis.yml to test on Racket 8.11, where a Racket compilation marshalling bug has been fixed.

  • Modify ac.rkt to use dynamic-require to get what it needs from the racket/random and openssl modules, so that we somehow manage to work around that marshalling bug on Racket versions where it exists.

What do you think? @hjek @akkartik

Member

rocketnia commented Feb 17, 2018

The error seems to be related to a bug in Racket that was introduced in version 6.3. I tried on several of the latest versions of Racket on Linux, and the most recent versions seem to work:

  • 6.8: Fails. (This is the version our Travis CI tests currently use.)
  • 6.9: Fails.
  • 6.10: Fails.
  • 6.10.1: Fails.
  • 6.11: Works.
  • 6.12: Works. (This is the version the Racket package index currently uses.)

Looking through Racket's commit history, I notice that indeed there's a fix for this just before version 6.11. (Hey, there's also a fix for text input streams on Windows! I hope that fixes the pasting-at-the-REPL issue.)

I tested with various old versions of Anarki too. It looks like this error started happening during my contributions last year, beginning with commit 2946803, which coincidentally(?) is the first commit where the Travis CI tests first started to work. This "lifted.0" error does sound distantly familiar... Maybe I saw it failing on the Racket package index at one point and just put it out of mind because I had no idea what to do about it.

I tinkered a little bit with that commit, and removing ac.scm's new dependency on racket/random seems to fix it. Back to the present-day state of the codebase, removing ac.rkt's dependencies on racket/random and openssl seems to fix it too. It's not as if we have to replace the functionality somehow, 'cause it seems to work if I replace ssl-connect with (dynamic-require 'openssl 'ssl-connect) and crypto-random-bytes with (dynamic-require 'racket/random 'crypto-random-bytes).

So it seems we have two pretty reasonable options here:

  • Change .travis.yml to test on Racket 8.11, where a Racket compilation marshalling bug has been fixed.

  • Modify ac.rkt to use dynamic-require to get what it needs from the racket/random and openssl modules, so that we somehow manage to work around that marshalling bug on Racket versions where it exists.

What do you think? @hjek @akkartik

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 17, 2018

Member

(I might just commit the dynamic-require fix in the morning to close the issue, and then we can potentially upgrade to 6.11 if that solution causes problems of its own.)

Member

rocketnia commented Feb 17, 2018

(I might just commit the dynamic-require fix in the morning to close the issue, and then we can potentially upgrade to 6.11 if that solution causes problems of its own.)

@akkartik

This comment has been minimized.

Show comment
Hide comment
@akkartik

akkartik Feb 17, 2018

Member

Thanks for the thorough investigation! I'd be fine with just fixing CI (since that seems easy) and modifying the Readme with the range of versions that cause one of the tests to fail. This is nobody's full-time job :) Do more only if you want to.

Member

akkartik commented Feb 17, 2018

Thanks for the thorough investigation! I'd be fine with just fixing CI (since that seems easy) and modifying the Readme with the range of versions that cause one of the tests to fail. This is nobody's full-time job :) Do more only if you want to.

rocketnia added a commit that referenced this issue Feb 17, 2018

Have ac.rkt depend on the `openssl` and `racket/random` libraries usi…
…ng `dynamic-require` rather than `require`. Somehow this avoids errors during `raco test --drdr` related to the unmarshaling of namespace information. Relates to #94, and might finally fix it for real this time.

rocketnia added a commit that referenced this issue Feb 17, 2018

Whoops, have arc.arc `dynamic-require` the `racket/random` library no…
…w that it's not part of ac.rkt's module namespace. Relates to #94, and may fix it.
@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 18, 2018

Member

Okay, I got Travis CI working. I used the dynamic-require approach because it was already on my mind.

I notice the Racket package index shows "build: succeeds" now. Success! I count that as enough to close this ticket, but I did a few more things to be sure:

I tested on several versions of Racket and determined the earliest one where the tests pass is now Racket 6.3 (much earlier than before). So I tweaked the Travis CI tests to test on two versions in particular: Racket 6.3 and Racket 6.12 (the current Racket version). That's represented in commit 21fec6a.

I developed a new implementation of arc.cmd on Windows so I could get run-news.cmd working (commit e7a44df). This new implementation contains almost no code that can't run equally well on Linux and OS X, so I hope it'll make it easier to maintain feature parity of the command-line interface without actually needing to test on Windows.

I tried the unit tests locally on Windows 10, and they needed no extra attention there. They're already passing!

So...

  • Unit tests are passing locally on Linux, and the run-news script works there.
  • They're passing locally on Windows 10, and run-news.cmd works there.
  • They're passing on Travis CI. Moreover, they're passing under multiple Racket versions, and we have notes on why those are the versions that work in particular. If a discrepancy between someone's local environment and Travis CI comes up again to create a build failure, I think we have a little more knowledge accessible to diagnose it with.
  • They're passing on the Racket package index. Moreover, Travis CI imitates the Racket package index faithfully enough now that I doubt we'll need to worry about checking the Racket package index much.

Looks like the tests pass in every way I can think of, and some of the discrepancies between those different tests are more streamlined now. I'm finally closing this ticket.

@akkartik I hear what you're saying: It's fine to put a note in the readme about Racket version incompatibilities if the fix is elusive. I think this time, I found all the fixes I needed. I nevertheless left notes in .travis.yml to explain why we seem to run up against various version limits in particular.

Member

rocketnia commented Feb 18, 2018

Okay, I got Travis CI working. I used the dynamic-require approach because it was already on my mind.

I notice the Racket package index shows "build: succeeds" now. Success! I count that as enough to close this ticket, but I did a few more things to be sure:

I tested on several versions of Racket and determined the earliest one where the tests pass is now Racket 6.3 (much earlier than before). So I tweaked the Travis CI tests to test on two versions in particular: Racket 6.3 and Racket 6.12 (the current Racket version). That's represented in commit 21fec6a.

I developed a new implementation of arc.cmd on Windows so I could get run-news.cmd working (commit e7a44df). This new implementation contains almost no code that can't run equally well on Linux and OS X, so I hope it'll make it easier to maintain feature parity of the command-line interface without actually needing to test on Windows.

I tried the unit tests locally on Windows 10, and they needed no extra attention there. They're already passing!

So...

  • Unit tests are passing locally on Linux, and the run-news script works there.
  • They're passing locally on Windows 10, and run-news.cmd works there.
  • They're passing on Travis CI. Moreover, they're passing under multiple Racket versions, and we have notes on why those are the versions that work in particular. If a discrepancy between someone's local environment and Travis CI comes up again to create a build failure, I think we have a little more knowledge accessible to diagnose it with.
  • They're passing on the Racket package index. Moreover, Travis CI imitates the Racket package index faithfully enough now that I doubt we'll need to worry about checking the Racket package index much.

Looks like the tests pass in every way I can think of, and some of the discrepancies between those different tests are more streamlined now. I'm finally closing this ticket.

@akkartik I hear what you're saying: It's fine to put a note in the readme about Racket version incompatibilities if the fix is elusive. I think this time, I found all the fixes I needed. I nevertheless left notes in .travis.yml to explain why we seem to run up against various version limits in particular.

@rocketnia rocketnia closed this Feb 18, 2018

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 18, 2018

Member

Oh yeah, the "lifted.0" error is the same one that came up in #62.

Member

rocketnia commented Feb 18, 2018

Oh yeah, the "lifted.0" error is the same one that came up in #62.

@akkartik

This comment has been minimized.

Show comment
Hide comment
@akkartik

akkartik Feb 18, 2018

Member

That's really perfect, thanks!! Very thorough docs in .travis.yml. I was trying to save you work, not add to your load ^_^ Most appreciated, @rocketnia.

(Everything's working for me as well.)

The Readme already says we require Racket v6.8 or later. So it shouldn't need any further change.

Member

akkartik commented Feb 18, 2018

That's really perfect, thanks!! Very thorough docs in .travis.yml. I was trying to save you work, not add to your load ^_^ Most appreciated, @rocketnia.

(Everything's working for me as well.)

The Readme already says we require Racket v6.8 or later. So it shouldn't need any further change.

@rocketnia

This comment has been minimized.

Show comment
Hide comment
@rocketnia

rocketnia Feb 18, 2018

Member

The Readme already says we require Racket v6.8 or later.

Oh, it does. XD I thought you might have been referring to something like that, but I specifically looked for a version number in the readme and didn't find it. I just managed to miss it I guess.

Member

rocketnia commented Feb 18, 2018

The Readme already says we require Racket v6.8 or later.

Oh, it does. XD I thought you might have been referring to something like that, but I specifically looked for a version number in the readme and didn't find it. I just managed to miss it I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment