search

greghendershott / rackjure

Provide a few Clojure-inspired ideas in Racket. Where Racket and Clojure conflict, prefer Racket.
BSD 2-Clause "Simplified" License
236 stars 17 forks source link

Test sometimes times out #55

Closed samth closed 8 years ago

samth commented 8 years ago

This test failure: http://pkg-build.racket-lang.org/server/built/test-fail/rackjure.txt (warning: rebuilds when the pkg changes) shows a timeout, probably because the futures test (or some other test) takes a long time.

greghendershott commented 8 years ago

The build log says:

utils.rkt: raco test: timeout after 90 seconds

I don't understand why the package server builder has a timeout that is (a) undocumented and (b) 3510 seconds shorter than Travis CI's one hour.

I think maybe the issue should be logged against the package server builder, instead? e.g. increase the timeout.

And also: Packages can have test modules that take a long time, do HTTP requests, or other things that are useful for testing that package. Is it right for the package server to effectively constrain these retroactively?

samth commented 8 years ago

The timeout is documented here: http://pkg-build.racket-lang.org/about.html

The limits are a lot lower than Travis because the system all runs on a single machine, rather than as a cloud service that people pay for.

The package server also prevents network access, etc. There's a discussion of handling these limitation on the above-linked page.

Perhaps @mflatt can say more about the pkg-build server, though.

greghendershott commented 8 years ago

I love the idea of a package build server to build documentation. And I also like the idea of it running tests. The question is, which tests.

I think the issue is that, prior to this, a package could use raco test . to run a variety of tests that are meaningful and useful to someone developing code for that package. But then the package build server comes along later and says, no, that should be a narrower set of tests. As a result, it makes it seem like the package needs to be fixed, which isn't necessarily true. The package might be fine. The package might even be getting a red badge of shame precisely because it was more ambitious about testing, which seems backwards.

Maybe one answer would be a package-server-omit-tests for info.rkt that allows narrowing what raco test . means on the package server. While allowing a broader set of tests to remain the default for package developers testing locally, as well as what counts as "OK to merge" for Travis CI run on pull requests, and so on.

samth commented 8 years ago

I created PR #56 to address the immediate issue.

As the pkg-build server docs describe, you can read an environment variable to disable some tests on the pkg-build server. For most testing issues (such as lack of network) I think the tests should detect the situation and not run themselves, but for timeouts that can be hard.

@mflatt, do you have any further thoughts?

greghendershott commented 8 years ago
  1. Thank you for the pull request. Merged! Sorry it took me so long to attend to it.
  2. The environment variable / conditional tests is a good point.

From having tried Travis CI and more recently Snap CI, I've noticed they share the convention to define an environment variable CI (as well as something specific to those services such as TRAVIS).

In addition to PLT_PKG_BUILD_SERVICE, maybe the Racket the pkg build server should also define CI? It might help simplify disabling certain tests that won't run on any external build system.