Stop looking for bundle.json if an invalid one is found #68
Conversation
srenatus
force-pushed
the
sr/stop-looking-for-bundle.json-if-one-is-found-but-invalid
branch
from
649be1d
to
e47c0c4
Compare
|
What you could do to test a package private class/actor/whateveryouwantbasically is to create a As an example: https://github.com/ponylang/http/blob/master/http/_test.pony and https://github.com/ponylang/http/blob/master/http/test/main.pony This way you can have both tests for private and public apis. |
|
@mfelsche thank you! I'll update this PR in a bit. (I had actually looked at that I generally wonder how we approach "hiding" in |
|
Don't merge Before: it would create one. Now: it fails. The |
|
✔️ This could be good to go. Since this is currently still lacking any integration-y tests, I suppose we could either scrutinize this PR thoroughly, or hold it off until we have some. On that topic, I'd be happy to help. It seems like #28 got a little stuck? ⏩ I could imagine either adding tests for |
|
@SeanTAllen I've split off getting integration test scaffolding in: #70 ☝️ |
|
@srenatus sounds good to me. |
…ent) This is in anticipation of what's going to change in ponylang#68. Signed-off-by: Stephan Renatus <[email protected]>
…ent) This is in anticipation of what's going to change in ponylang#68. Signed-off-by: Stephan Renatus <[email protected]>
|
@srenatus this needs to be rebased. |
* Run integration tests in their own temporary directory
Having not found a way to make `@chdir[I32]` or anything like that work,
this commit adds a bash helper script that `cd`s into the directory
before running stable.
Also includes the code changes necessary to create the temporary dir
from pony, and remove it after the tests are done.
The former is done so that upcoming tests can use pony to set up some
initial situation (such as drop a bundle.json).
The cleanup is not done in a `tear_down()` method, because I found no
non-clumsy way to get the reference to the directory into that spot. So
instead, this has a mini-actor that deletes the temp dir when disposed,
and is set up to be called by the test helper.
Note that there's no strict need to run the existing integration tests
in their own temp dirs, so we might make that temp dir passing optional,
maybe, by changing `Exec`'s `cleaner arg from
cleaner: DisposableActor
to
cleaner: (DisposableActor | None)
and pass None for help and version.
* Add integration tests for 'stable env'
Turns out this is probably the lowest-hanging fruit right now. This makes
use of the previous commit's "run in temp dir" feature.
* Add test case for invalid bundle.json in nested dir (valid one in parent)
This is in anticipation of what's going to change in #68.
* Run integration tests in separate tmp directory
This adds stable/test/integration/tmp/ as an empty, tracked folder; and
uses that one as base directory for running the integration tests.
It felt a bit cleaner to me than having the temp dirs created in the
top-level folder; also causes less of a mess when debugging integration
tests by disabling the cleanup dispose()'s delete call.
(I suppose it would have been just as good to use /tmp/, but I wasn't
sure what our expectations regarding different operating systems were.)
* Run integration tests sequentially
One of these tests had timed out on circleci. Having flakey tests seems
an annoyance we should try to avoid; but I couldn't reproduce this by
running `make integration` in a loop locally. So, the hypothesis is that
the timeout was no bug in the tests, but a genuine timeout.
Trying to reproduce the test failures locally, I've found that
restricting the memory to 4M (docker's minimum) caused the tests to fail
in a reproducible way.
The integration test binary is now passed `--sequential`, so that the
individual test runs don't compete for resources.
This makes the error _not appear anymore_ in the docker 4M scenario.
This reverts commit 8d8a906.
* Double integration test timeouts
Seems like --sequential alone isn't enough. 200ms doesn't seem too bad,
so let's try this.
* integration: register tmpdir cleanup after creation
* integration/helper.sh: mention chdir issue
Signed-off-by: Stephan Renatus <[email protected]>
The previous behaviour was a little irritating:
If invoked in /some/nested/dir, stable would try to read
/some/nested/dir/bundle.json
and if that failed (because the file didn't exist or was invalid), it
would try to read, subsequently,
/some/nested/bundle.json
/some/bundle.json
/bundle.json
This implied that errors reading any found bundle.json were printed, but
stable kept trying to find a valid one, and eventually, having reached
/bundle.json and assuming it didn't exist, would print
No bundle.json in current working directory or ancestors.
With this change, the logic is different: stable will first try to find
an existing bundle.json, and then attempt to read it. If it fails, it
will not go on, but stop.
Signed-off-by: Stephan Renatus <[email protected]>
I'm not sure if this is the best approach, but it seems using primitives for this is valid (judging from the patterns docs). Signed-off-by: Stephan Renatus <[email protected]>
Signed-off-by: Stephan Renatus <[email protected]>
Signed-off-by: Stephan Renatus <[email protected]>
Signed-off-by: Stephan Renatus <[email protected]>
srenatus
force-pushed
the
sr/stop-looking-for-bundle.json-if-one-is-found-but-invalid
branch
from
04a3b79
to
3dd2acb
Compare
|
@mfelsche rebased and updated ✔️ |
|
booyah! |
srenatus
deleted the
sr/stop-looking-for-bundle.json-if-one-is-found-but-invalid
branch
The previous behaviour was a little irritating:
If invoked in /some/nested/dir, stable would try to read
and if that failed (because the file didn't exist or was invalid), it
would try to read, subsequently,
This implied that errors reading any found bundle.json were printed, but
stable kept trying to find a valid one, and eventually, having reached
/bundle.json and assuming it didn't exist, would print
With this change, the logic is different: stable will first try to find
an existing bundle.json, and then attempt to read it. If it fails, it
will not go on, but stop.
Questions
Q: How to test this?
stablebinary given different input situations could include a case of this "nested with broken bundle.json at the bottom" test._locate_bundle()alone could be tested if it was split off theMainactor. Is there a pattern to follow for this?Bundlewhat would be able to differentiate "keep going" from "stop it" errors. (Where an existing file with a bad format would trigger a stop; and no-file-found would make it "keep going".)A: I went with the last option, introducing a
primitive BundleLocator, and adding tests for that. I've failed to hide it better, as calling it_BundleLocatormade it impossible to import it in tests, it seems?