Project

General

Profile

Actions

Feature #14187

closed

`make test` and `make check` to run all test suites

Added by mame (Yusuke Endoh) over 6 years ago. Updated over 5 years ago.

Status:
Closed
Assignee:
-
Target version:
[ruby-core:84275]

Description

Currently, MRI has many test suites: "bootstraptest/", "KNOWNBUGS.rb", "basictest/", "test/", and "spec/". And, the target name design of make is really complicated:

  • make test runs the first three suites
  • make test-all runs only test/
  • make check runs all the suites except spec/
  • make exam runs all the suites

I think when a casual user build and install ruby, s/he will expect make test or make check to run all recommended test suites because many other projects uses them as the semantics.

Thus, how about changing make test and make check to run all test suites?

I think this change will affect some ruby development tools including rubyci (chkbuild), mswin-build, and ruby-loco. @hsbt (Hiroshi SHIBATA), @usa (Usaku NAKAMURA), @MSP-Greg (Greg L), what do you think?

diff --git a/common.mk b/common.mk
index 80001b481c..ba5fffdf4b 100644
--- a/common.mk
+++ b/common.mk
@@ -634,10 +634,6 @@ clean-spec: PHONY
 	-$(Q) $(RM) $(RUBYSPEC_CAPIEXT)/*.$(OBJEXT) $(RUBYSPEC_CAPIEXT)/*.$(DLEXT)
 	-$(Q) $(RMDIRS) $(RUBYSPEC_CAPIEXT) 2> $(NULL) || exit 0
 
-check: main test test-testframework test-almost
-	$(ECHO) check succeeded
-check-ruby: test test-ruby
-
 fake: $(CROSS_COMPILING)-fake
 yes-fake: $(arch)-fake.rb $(RBCONFIG) PHONY
 no-fake -fake: PHONY
@@ -660,6 +656,8 @@ no-btest-ruby: PHONY
 yes-btest-ruby: prog PHONY
 	$(Q)$(exec) $(RUNRUBY) "$(srcdir)/bootstraptest/runner.rb" --ruby="$(PROGRAM) -I$(srcdir)/lib $(RUN_OPTS)" -q $(OPTS) $(TESTOPTS)
 
+test-bootstrap: btest-ruby
+
 test-basic: $(TEST_RUNNABLE)-test-basic
 no-test-basic: PHONY
 yes-test-basic: prog PHONY
@@ -677,7 +675,6 @@ yes-test-testframework: prog PHONY
 no-test-testframework: PHONY
 
 test-sample: test-basic # backward compatibility for mswin-build
-test: btest-ruby test-knownbug test-basic
 
 # $ make test-all TESTOPTS="--help" displays more detail
 # for example, make test-all TESTOPTS="-j2 -v -n test-name -- test-file-name"
@@ -755,7 +752,7 @@ $(ENC_MK): $(srcdir)/enc/make_encmake.rb $(srcdir)/enc/Makefile.in $(srcdir)/enc
 .PHONY: clean clean-ext clean-local clean-enc clean-golf clean-rdoc clean-html clean-extout
 .PHONY: distclean distclean-ext distclean-local distclean-enc distclean-golf distclean-extout
 .PHONY: realclean realclean-ext realclean-local realclean-enc realclean-golf realclean-extout
-.PHONY: check test test-all btest btest-ruby test-basic test-knownbug
+.PHONY: exam check test test-all test-bootstrap btest btest-ruby test-basic test-knownbug
 .PHONY: run runruby parse benchmark benchmark-each tbench gdb gdb-ruby
 .PHONY: update-mspec update-rubyspec test-rubyspec test-spec
 .PHONY: touch-unicode-files
@@ -1322,9 +1319,12 @@ info-arch: PHONY
 change: PHONY
 	$(BASERUBY) -C "$(srcdir)" ./tool/change_maker.rb $(CHANGES) > change.log
 
-exam: check test-spec
+test: main test-bootstrap test-knownbug test-basic test-testframework test-almost test-spec
+	$(ECHO) check succeeded
+check: test
+exam: test
 
-love: sudo-precheck up all test install exam
+love: sudo-precheck up all test install
 	@echo love is all you need
 
 great: exam
@@ -1356,11 +1356,11 @@ help: PHONY
 	"  runruby:             runs test.rb by ruby you just built" \
 	"  gdb:                 runs test.rb by miniruby under gdb" \
 	"  gdb-ruby:            runs test.rb by ruby under gdb" \
-	"  check:               equals make test test-all" \
-	"  exam:                equals make check test-spec" \
-	"  test:                ruby core tests" \
-	"  test-all:            all ruby tests [TESTOPTS=-j4 TESTS=<test files>]" \
-	"  test-spec:           run the Ruby spec suite" \
+	"  test:                ruby all test suites" \
+	"  check:               equals make test" \
+	"  exam:                equals make test" \
+	"  test-all:            run `test/` suite [TESTOPTS=-j4 TESTS=<test files>]" \
+	"  test-spec:           run the Ruby spec suite (in `spec/`)" \
 	"  test-rubyspec:       same as test-spec" \
 	"  test-bundler:        run the Bundler spec" \
 	"  test-bundled-gems:   run the test suite of bundled gems" \

Updated by usa (Usaku NAKAMURA) over 6 years ago

Yes, I (mswin-build) will be critically affected by this change.
But I'll be able to fix it.
If you really believe that this is necessary, I'll not oppose.

My only hope is that the timing of the change is appropriate.
For example, just after changing version number.

Updated by Eregon (Benoit Daloze) over 6 years ago

This looks good to me.
I think we should still have a way to run each of the 3 groups individually: "basic tests", test-all and test-spec.
I often run test/ and spec/ individually while working on MRI.
Specifically, I think test-all should not not include the "basic tests" (as it is currently, right?).

test-all still has a confusing name, but that seems difficult to change.
They are often referred as "MRI tests" in other implementations.

Updated by MSP-Greg (Greg L) over 6 years ago

mame (Yusuke Endoh) wrote:

I think when a casual user build and install ruby, s/he will expect make test or make check to run all recommended test suites because many other projects uses them as the semantics.

Eregon (Benoit Daloze) wrote:

I think we should still have a way to run each of the 3 groups individually: "basic tests", test-all and test-spec.

usa (Usaku NAKAMURA) wrote:

For example, just after changing version number.

I agree with all of the above.

Two other thoughts:

  1. Now, or in the future, should test-bundler and/or test-bundled-gems be included in 'global' targets?

  2. I've got a Travis repo https://travis-ci.org/MSP-Greg/travis-ruby, and an Appveyor repo https://ci.appveyor.com/project/MSP-Greg/appveyor-ruby that I created just to check information on ruby builds. Might a report like I've used in these be helpful as a check to 'casual' builders? The Travis builds have always been something that I would not consider a proper build. BTW, the report is designed for a new build, it gets a bit cluttered when run on a day-to-day ruby with a lot gems...

You've used the term 'casual'; wish I could think of a better term. Regardless, I think there are many people building ruby (packagers, etc), that may not realize that their build doesn't include what is expected, or is somehow structured incorrectly.

Thanks, Greg

Updated by duerst (Martin Dürst) over 6 years ago

I think it would be good to have a table for this proposal, with three columns:

  1. What is tested
  2. What is the target name currently
  3. What is the proposed new target name
    That will be much easier to understand than a lengthy description.

Updated by mame (Yusuke Endoh) almost 6 years ago

  • Target version set to 2.7

Current test suites:

  • bootstraptest/: Some trivial tests to verify the VM; each test is executed under a new Ruby process because the failure easily leads to a critical error such as a segfault.
  • KNOWNBUGS.rb: Some test to keep known issues; usually empty.
  • basictest/: One-file test once called test.rb; this is the oldest test for Ruby.
  • test/: One of the main test suites; this is written in minitest.
  • spec/: Another main test suite (or spec?) once called rubyspec.

The current relation between test suites and target names:

suite\target test test-all check exam
bootstraptest/ o o o
KNOWNBUGS.rb o o o
basictest/ o o o
test/ o o o
spec/ o

I'd like to change this to:

suite\target test test-all check exam test-basic
bootstraptest/ o o o o
KNOWNBUGS.rb o o o o
basictest/ o o o o
test/ o o o o
spec/ o o o

I've added make test-basic that is equivalent to the old make test.

If there is no objection, I'd like to introduce this change immediately after Ruby 2.6 is released.

Updated by naruse (Yui NARUSE) almost 6 years ago

Casual users really needs to run test-all and rubyspec?
Those tests often fail on some environment. (see also https://rubyci.org/)
It will be false positive for casual users.

Anyway I agree with adding also rubypec to make check.

Updated by Eregon (Benoit Daloze) almost 6 years ago

Looks good to me, +1.

I don't know who uses just make test currently.
But if it's to check changes to CRuby then I think that's too little, and the proposal seems better.

spec/: Another main test suite (or spec?) once called rubyspec.

"Another main test suite called ruby/spec" sounds fine :)

Updated by hsbt (Hiroshi SHIBATA) almost 6 years ago

+1

I'd prefer mame's change.

Updated by mame (Yusuke Endoh) almost 6 years ago

naruse (Yui NARUSE) wrote:

Casual users really needs to run test-all and rubyspec?

The short answer is "I think so".

I agree that there is many "levels" of test suites: easy tests for casual users to verify their installation, daily tests for developers to verify their changes, exhaustive tests for CI to verify abnormal handling code like out of memory, etc. However, there is currently no consensus about the relationship between the levels and the test suites. If you reorganize (or split) the test suites into each level, it would be a good idea to assign make test only to "easy test for casual users". But currently there is no such levels. To avoid missing an issue, it would be good for casual users to run all, I think.

By the way, we cannot use the name test-basic because there has been already make test-basic to run basictest/. Is there any other idea?

Updated by MSP-Greg (Greg L) almost 6 years ago

Messy issue, especially taking into account whether the user downloads a pre-built install, or builds their own. Without a build/compile system, tests can't be run with make...

Having paid attention to various repos for popular gems, a casual user can very easily install gems that expect a fully working Ruby install. Hence, I agree that casual users may have a need to run test-all & spec.

Re ruby-loco, I don't recall ever having problems in bootstraptest or basictest. Spec rarely fails (thanks @Eregon (Benoit Daloze)), but one could say it is somewhat more 'platform independent' than test-all.

Is there any other idea?

Wish I had a good answer. I don't. I run the tests separately so I can split the logs, but for casual users, I think the suggestion should be to run all the tests.

I keep thinking of taking one's car in for repair, and the service manager says 'it passed all the quick/basic tests'...

Updated by ioquatix (Samuel Williams) over 5 years ago

I've been working on PRs for MRI, and I got bitten by make test not running all tests. I wish it was the simple default, make test should run all tests. If users want something else, make more specific targets, e.g. make test-core etc.

Actions #12

Updated by mame (Yusuke Endoh) over 5 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r64270.


common.mk: "make check" now runs test-spec

Currently there are many "make" targets for testing: test, test-all,
check, exam, etc.
To make it simple, this change makes "make check" run all tests.
"make exam" is just an alias to "make check".
If a new test suite is added in future, "make check" should include it
(unless it takes too much time...)
[Feature #14187]

Also, this introduces "make test-short" as an alias to "make test".
I believe "make test" should equal to "make check", but there is
objection against this. So now I commit only things that we agreed.

Updated by mame (Yusuke Endoh) over 5 years ago

  • Status changed from Closed to Open

I changed "make check" to run all tests. But I don't change "make test" yet because of naruse's objection. So I keep this ticket open.

ioquatix (Samuel Williams) wrote:

I've been working on PRs for MRI, and I got bitten by make test not running all tests. I wish it was the simple default, make test should run all tests. If users want something else, make more specific targets, e.g. make test-core etc.

Agreed.

Updated by naruse (Yui NARUSE) over 5 years ago

ioquatix (Samuel Williams) wrote:

I've been working on PRs for MRI, and I got bitten by make test not running all tests. I wish it was the simple default, make test should run all tests. If users want something else, make more specific targets, e.g. make test-core etc.

test-all includes many tests may fail.
For example recent tzdata update breaks tests: https://rubyci.org/logs/rubyci.s3.amazonaws.com/centos6/ruby-2.3/log/20180810T073929Z.fail.html.gz
Many users who tries to run tests cannot understand why this fails and resolve this.
Other tests related to IPv6, /etc/hosts, firewall, and so on also depend on the environment.

If you don't know make test-all yet, you are not CRuby developer yet.
And now you know test-all and other test targets, welcome to CRuby developer.

Actions #15

Updated by mame (Yusuke Endoh) over 5 years ago

  • Status changed from Open to Closed

Applied in changeset commit:ruby-git|163b830d70dabe629816bf7ecf8e5ad3bd511c3e.


common.mk: "make check" now runs test-spec

Currently there are many "make" targets for testing: test, test-all,
check, exam, etc.
To make it simple, this change makes "make check" run all tests.
"make exam" is just an alias to "make check".
If a new test suite is added in future, "make check" should include it
(unless it takes too much time...)
[Feature #14187]

Also, this introduces "make test-short" as an alias to "make test".
I believe "make test" should equal to "make check", but there is
objection against this. So now I commit only things that we agreed.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64270 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0