MagLev / maglev

GemStone Maglev Ruby Repository
http://maglev.github.io
517 stars 41 forks source link

Exposes Object#become #440

Closed johnnyt closed 9 years ago

johnnyt commented 9 years ago

Closes #439.

timfel commented 9 years ago

It'd be great to add a test.

johnnyt commented 9 years ago

@timfel - where do you think the test should live - somewhere in src/test/ObjectTest.rb?

johnnyt commented 9 years ago

@jc00ke or @timfel - thoughts on this?

jc00ke commented 9 years ago

Personally I'd like to see the tests in src/test be cleaned up/removed and we use RubySpec and specs defined in spec/maglev for our own things exposed.

I'm guessing there's a lot of duplication between src/test and RubySpec. Seems like anything else should be in spec/maglev to me.

johnnyt commented 9 years ago

I also like the idea of using spec/maglev over src/test.

What do you think about merging this in? I'll be talking about this in my presentation at LARuby Conf on Saturday - and would love to have it exposed by then.

AllenOtis commented 9 years ago

The tests in src/test are essential to debugging changes to the VM and should not be deleted. While there might be some duplication with other tests, I would say at least 50% of the tests need to be kept. It's too complicated to debug VM changes by just running specs because of the amount of ruby code in the test framework. After the simple ruby programs in src/test work, then it's feasible to run spec tests.

jc00ke commented 9 years ago

Shouldn't they just be added for debugging and then removed if the fix then passes a corresponding RubySpec?

I see the utility in src/test but it seems temporal to me. Once fixed, dump 'em?

AllenOtis commented 9 years ago

We need the tests in src/test kept as a regression test of the VM , so that when major changes are made to Smalltalk bytecodes or to the in memory GC , we have simple test cases with which do debug any Ruby-specific problems at the bytecode level .

For example, in the GemStone server v3.3 the block execution bytecodes are drastically changed at the VM level, and there are some Ruby-specific code paths in that VM that I have not yet debugged. There is too much complicated ruby code in the spec framework; figuring out what is wrong with an individual bytecode is way simpler with the tests in src/test .

On Thu, Oct 8, 2015 at 11:06 AM, Jesse Cooke notifications@github.com wrote:

Shouldn't they just be added for debugging and then removed if the fix then passes a corresponding RubySpec?

I see the utility in src/test but it seems temporal to me. Once fixed, dump 'em?

— Reply to this email directly or view it on GitHub https://github.com/MagLev/maglev/pull/440#issuecomment-146640674.

jc00ke commented 9 years ago

Can we consolidate them at least at a directory level? I think it's confusing to have both, especially given the vast majority of Ruby projects use a top-level spec/ or test/.

AllenOtis commented 9 years ago

The could be moved to a test/vmunit or spec/vmunit or similar directory. Just make sure they stay as simple ruby main programs, without being executed by a test framework.

On Thu, Oct 8, 2015 at 11:38 AM, Jesse Cooke notifications@github.com wrote:

Can we consolidate them at least at a directory level? I think it's confusing to have both, especially given the vast majority of Ruby projects use a top-level spec/ or test/.

— Reply to this email directly or view it on GitHub https://github.com/MagLev/maglev/pull/440#issuecomment-146648453.

jc00ke commented 9 years ago

:+1: that would make me :smile: