This repository has been archived by the owner. It is now read-only.

Isolate Travis::API specs #78

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
@sarahhodne
Contributor

sarahhodne commented Jul 15, 2012

This shaves off about 0.8 seconds from the test time with warm caches, and about two seconds with cold caches (disk cache not included).

Is this something we want to do? I could go ahead and isolate even more things, but if it's not wanted then I won't bother.

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 15, 2012

This pull request fails (merged 1b540796 into 3dc56d4).

This pull request fails (merged 1b540796 into 3dc56d4).

@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 15, 2012

Contributor

Oops.

Contributor

sarahhodne commented Jul 15, 2012

Oops.

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 15, 2012

This pull request fails (merged 8631a1b0 into 3dc56d4).

This pull request fails (merged 8631a1b0 into 3dc56d4).

@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 15, 2012

Contributor

The specs fail due to some odd bug. If I run them locally with bundle exec rspec spec/**/*_spec.rb, they pass. If I run bundle exec rake, they fail. Only difference is the order the spec files are run in.

Contributor

sarahhodne commented Jul 15, 2012

The specs fail due to some odd bug. If I run them locally with bundle exec rspec spec/**/*_spec.rb, they pass. If I run bundle exec rake, they fail. Only difference is the order the spec files are run in.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Jul 16, 2012

Member

@henrikhodne if there is an ordering issue then maybe you could try and fix that as well?

Member

joshk commented Jul 16, 2012

@henrikhodne if there is an ordering issue then maybe you could try and fix that as well?

@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 16, 2012

Contributor

@joshk I'm working on it.

Contributor

sarahhodne commented Jul 16, 2012

@joshk I'm working on it.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Jul 16, 2012

Member

❤️ ❤️ ❤️

On 16/07/2012, at 4:15 PM, Henrik Hodne wrote:

@joshk I'm working on it.


Reply to this email directly or view it on GitHub:
#78 (comment)

Member

joshk commented Jul 16, 2012

❤️ ❤️ ❤️

On 16/07/2012, at 4:15 PM, Henrik Hodne wrote:

@joshk I'm working on it.


Reply to this email directly or view it on GitHub:
#78 (comment)

@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 17, 2012

Contributor

Ok, I found the problem. When api_spec.rb is run, it creates the Repository class. This means that rails won't autoload it. So, in short, this can't be done unless we separate specs into specs that can be run without rails and specs that have to be run with rails and then run them separately. At least that's all I can think of.

Contributor

sarahhodne commented Jul 17, 2012

Ok, I found the problem. When api_spec.rb is run, it creates the Repository class. This means that rails won't autoload it. So, in short, this can't be done unless we separate specs into specs that can be run without rails and specs that have to be run with rails and then run them separately. At least that's all I can think of.

Stub the Repository class
The api_spec.rb file is ran first on some ruby implementations/systems,
and if we create the Repository class there, that messes up Rails'
autoloading in later tests.
@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 18, 2012

Contributor

This pull request passes (merged 73b6ee0 into 2531cab).

Contributor

sarahhodne commented Jul 18, 2012

This pull request passes (merged 73b6ee0 into 2531cab).

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Jul 18, 2012

Member

:/
why are you commenting like travisbot comments?

Member

joshk commented Jul 18, 2012

:/
why are you commenting like travisbot comments?

@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 18, 2012

Contributor

Because the job got stuck, but the entire matrix passed (check the log messages).

Contributor

sarahhodne commented Jul 18, 2012

Because the job got stuck, but the entire matrix passed (check the log messages).

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 18, 2012

This pull request passes (merged 73b6ee0 into 2531cab).

This pull request passes (merged 73b6ee0 into 2531cab).

@sarahhodne

This comment has been minimized.

Show comment
Hide comment
@sarahhodne

sarahhodne Jul 18, 2012

Contributor

I stand corrected… Sorry about that.

Contributor

sarahhodne commented Jul 18, 2012

I stand corrected… Sorry about that.

+require 'travis/api'
+
+RSpec.configure do |c|
+ c.mock_with :mocha

This comment has been minimized.

@dmathieu

dmathieu Aug 21, 2012

Contributor

What about not using mocha (in relation to #93) ?

@dmathieu

dmathieu Aug 21, 2012

Contributor

What about not using mocha (in relation to #93) ?

This comment has been minimized.

@rkh

rkh Aug 21, 2012

Member

I wouldn't mind moving away from mocha. I think the tests need some rewriting though.

@rkh

rkh Aug 21, 2012

Member

I wouldn't mind moving away from mocha. I think the tests need some rewriting though.

@sarahhodne sarahhodne closed this Dec 12, 2012

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