Skip to content

Commit

Permalink
[Truffle] JT: refactor a bit the logic in the test command.
Browse files Browse the repository at this point in the history
@eregon
eregon committed May 24, 2015
1 parent bd7c866 commit 70f7bf6
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions tool/jt.rb
View file
Original file line number Diff line number Diff line change
@@ -268,15 +268,16 @@ def test_mri(*args)
private :test_mri

def test(*args)
return test_pe(*args.drop(1)) if args.first == 'pe'
return test_mri(*args.drop(1)) if args.first == 'mri'
return test_specs(*args.drop(1)) if args.first == 'specs'
path, *rest = args

if args.empty?
test_specs(*args)
test_mri(*args)
case path
when nil
test_specs
test_mri
when 'pe' then test_pe(*rest)
when 'specs' then test_specs(*rest)
when 'mri' then test_mri(*rest)
else
path = args.first
if File.expand_path(path).start_with?("#{JRUBY_DIR}/test")
test_mri(*args)
else

7 comments on commit 70f7bf6

@bjfish
Copy link
Contributor

@bjfish bjfish commented on 70f7bf6 May 25, 2015

Choose a reason for hiding this comment

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

In one of the jt test refactorings, I think we can no longer run an MRI test like this:
jt test digest/test_digest.rb. I think I forgot to include an example of this in the help section before.

@eregon
Copy link
Member Author

@eregon eregon commented on 70f7bf6 May 26, 2015

Choose a reason for hiding this comment

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

Yes, I am aware of the problem. @chrisseaton and I use mostly tab completion to run tests/specs so we did not run into the problem.
Of course, jt test mri digest/test_digest.rb would just work.

I was thinking maybe to have jt test for MRI test suite and jt spec for RubySpec, but then how do we name the next suite we use?

We could try guessing from the path, but with a partial path like this it gets confusing.
And RubySpec also has the feature to look paths you give to it with an implicit prefix, although we do not currently use it. Also, we would need to parse options somehow to get the path in jt test --jdebug .../some_spec.rb
Supporting single files should be easy (test_... vs ..._spec.rb) but how to deal with directories for instance? (jt test mri digest vs jt test specs library/digest?)

@chrisseaton @nirvdrum I am not sure how smart we want to be with this, do we want to go down the road of trying to guess the appropriate runner given a path, or do we prefer to keep things simple but have to know the different jt commands?
I am definitely not a big fan of subcommands (jt test mri|specs) in this case.

@chrisseaton
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer guessing the right kind of test to run. I don't really understand why people would want to write a partial file path like digest/test_digest.rb - that's not how any other programming tool I know works. If we require full paths I also can't imagine any ambiguity.

@eregon
Copy link
Member Author

@eregon eregon commented on 70f7bf6 May 26, 2015

Choose a reason for hiding this comment

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

So, should we support jt test digest/test_digest.rb ?
And not just jt test test/mri/digest/test_digest.rb?

@chrisseaton
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think so. I don't understand why anyone would expect the short version to work - is that how the MRI test running usually does things?

@bjfish
Copy link
Contributor

@bjfish bjfish commented on 70f7bf6 May 26, 2015

Choose a reason for hiding this comment

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

As far as I can tell, the partial paths are just how MRI tests work. I usually copy the path I want from the mri_truffle.index.

I don't have a preference about which format we support. We could even use both if you'd prefer with something like:

if path.starts_with("test/mri/")
  path.slice! "test/mri/"
end

It would be nice to have tab completion sometimes.

@eregon
Copy link
Member Author

@eregon eregon commented on 70f7bf6 May 26, 2015

Choose a reason for hiding this comment

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

The MRI tests run just fine with the full path, right?
In that case I propose to start using the full path and then report if there is any problem with that.

Please sign in to comment.