syberia / modeling.sy

The core modeling Syberia engine
MIT License
4 stars 1 forks source link

remove SyberiaStages dependency + fix Travis #8

Closed abelcastilloavant closed 8 years ago

abelcastilloavant commented 8 years ago

All tests pass when using test_engine locally. @robertzk ideas on what might have gone wrong?

abelcastilloavant commented 8 years ago

Error message from Travis suggest problems with the methods package:

Running tests... simpleError in if (rep$context_open) { rep$end_context()} else { rep$context_open <- TRUE}: argument is of length zero Warning messages: 1: 'package:methods' may not be available when loading 2: 'package:methods' may not be available when loading 3: 'package:methods' may not be available when loading 4: 'package:methods' may not be available when loading 5: 'package:methods' may not be available when loading

peterhurford commented 8 years ago

@abelcastilloavant - Travis is still pretty buggy for this, but we're working on it.

abelcastilloavant commented 8 years ago

Does that mean this is ready to merge? @peterhurford

peterhurford commented 8 years ago

Wait for @robertzk to review and approve.

robertzk commented 8 years ago

Trying to get the tests working.

abelcastilloavant commented 8 years ago

Travis is reporting a success, even though there was an error during the run in: https://travis-ci.org/syberia/modeling.sy/builds/124830292#L493

lib/adapters/s3: Error in library(s3mpi) : there is no package called ‘s3mpi’

The attempt to load the library comes from here: https://travis-ci.org/syberia/modeling.sy/builds/124830292#L547

In /home/travis/build/syberia/modeling.sy/test/lib/adapters/s3.R:3: library(s3mpi)

peterhurford commented 8 years ago

Oh yeah @robertzk and I never did fix that thing we did to Travis to make it always report success 😨

peterhurford commented 8 years ago

Hold off on merging; I'll fix in another PR.

peterhurford commented 8 years ago

@abelcastilloavant Just put this change https://github.com/syberia/modeling.sy/pull/13 in your branch and I'll close my PR. Looks like I'm also having the same s3mpi issue.

abelcastilloavant commented 8 years ago

@peterhurford done.

peterhurford commented 8 years ago

Nice. Now add s3mpi to the lockfile: https://github.com/syberia/modeling.sy/blob/master/lockfile.yml

peterhurford commented 8 years ago

and revert https://github.com/syberia/modeling.sy/pull/8/commits/c4e797707831898acc844a53e73f3f7f8ff595e6

peterhurford commented 8 years ago

...Looks like AWS.tools went off CRAN again. What a weird package.

abelcastilloavant commented 8 years ago

I was about to say. Does this mean adding it as a remote to s3mpi?

peterhurford commented 8 years ago

@abelcastilloavant Yep. https://github.com/robertzk/s3mpi/pull/45

peterhurford commented 8 years ago

@abelcastilloavant Ready

abelcastilloavant commented 8 years ago

Woohoo, tests pass! Now to remove the SyberiaStages package.

peterhurford commented 8 years ago

Let's wait for @robertzk to LGTM and then merge. Looks good to me, though.

peterhurford commented 8 years ago

@abelcastilloavant Try taking out syberiaStructure too and see if tests still pass.

peterhurford commented 8 years ago

@abelcastilloavant Awesome that tests pass!

robertzk commented 8 years ago

Thanks!