Skip to content

Conversation

@jvanbaarsen
Copy link
Contributor

This will make sure the specs are beeing run again. since my fix in jvanbaarsen@2314438 broke this..

ping @randx

Signed-off-by: Jeroen van Baarsen <jeroenvanbaarsen@gmail.com>
@jvanbaarsen jvanbaarsen mentioned this pull request Jun 6, 2014
@dzaporozhets
Copy link
Contributor

@jvanbaarsen ping me when tests are greeen. No merge with broken tests any more :)

@jvanbaarsen
Copy link
Contributor Author

Thats the problem with this fix, tests wont be green since the weird EOF error. So that means you wont be able to accept community MRs again. Please reconsider.—
Sent from Mailbox

On Fri, Jun 6, 2014 at 4:55 PM, Dmitriy Zaporozhets
notifications@github.com wrote:

@jvanbaarsen ping me when tests are greeen. No merge with broken tests any more :)

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

@dzaporozhets
Copy link
Contributor

@jvanbaarsen ok so what use of accepting this if we get always failing tests?

@jvanbaarsen
Copy link
Contributor Author

Ill give you an extended reply later tonight, ok?—
Sent from Mailbox

On Fri, Jun 6, 2014 at 5:04 PM, Dmitriy Zaporozhets
notifications@github.com wrote:

@jvanbaarsen ok so what use of accepting this if we get always failing tests?

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

@jvanbaarsen
Copy link
Contributor Author

Oh boy, im really screwing this up... I see what i've done... Ill take some time to fix this, and not rush this, since im only making this worse it seems...

Signed-off-by: Jeroen van Baarsen <jeroenvanbaarsen@gmail.com>
@dzaporozhets
Copy link
Contributor

@jvanbaarsen ok thanks ;)

@jvanbaarsen
Copy link
Contributor Author

@randx Let me try to explain the problem with the first fix i've proposed (the one that i wanted to have reverted).

I've changed some stuff in db_cleaner to be an around block. But the problem with the around block, is that you have to give it a block, with the example as param and call the example with example.run.

I had not done that, that way, all the rspec tests are currently not ran (it seems like they run, but in fact, they did not run). i cam to this conclusion when i was trying to create a new feature, and whatever i did, the tests where green. See rspec/rspec-core#1581 for more info. So at this point im trying to figure out, how to get the DB cleaner to be in an around block, but execute specs as they should.. So thats where all the messing stuff up started :( So it can be, that after i've finally fix this problem, some tests are red. Since they where probably red since i created the rspec fix, but just now they got run :(

Again sorry for all the trouble, im trying as much as i can to solve this issue.

Signed-off-by: Jeroen van Baarsen <jeroenvanbaarsen@gmail.com>
Signed-off-by: Jeroen van Baarsen <jeroenvanbaarsen@gmail.com>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the new Ruby 1.9 hash syntax.

@dzaporozhets
Copy link
Contributor

@jvanbaarsen ping me when ready

@jvanbaarsen
Copy link
Contributor Author

@randx Ok, what about the current failing specs (The ones that have not run) shall i also fix those in this PR?

@dzaporozhets
Copy link
Contributor

@jvanbaarsen ok it looks much better now. Looks like current failing tests is a result of no-rspec during last week. We can merge this PR and start fixing tests in other PR's

dzaporozhets added a commit that referenced this pull request Jun 7, 2014
Temp fix for rspec so the specs are run again
@dzaporozhets dzaporozhets merged commit 7a89cef into gitlabhq:master Jun 7, 2014
@jvanbaarsen
Copy link
Contributor Author

@randx Ok, ill start fixing bugs in about an hour, have to finish some other stuff first..

@dzaporozhets
Copy link
Contributor

@jvanbaarsen ok and I will fix rest at monday I think

@jvanbaarsen jvanbaarsen deleted the tmp-fix-for-rspec branch July 16, 2014 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants