Allow new-run to run tests and benchmarks too #4861

fgaz · 2017-11-03T12:33:47Z

This is especially useful for passing arguments to a single test suite

ping @merijn this is already usable

phadej · 2017-11-03T12:37:38Z

cabal-install/Distribution/Client/CmdRun.hs

@@ -309,11 +311,11 @@ selectPackageTargets targetSelector targets
  where
    (targetsExesBuildable,
     targetsExesBuildable') = selectBuildableTargets'
-                            . filterTargetsKind ExeKind
+                            . (\ts -> concatMap (`filterTargetsKind` ts) [ExeKind, TestKind, BenchKind])


ExeKind was first argument of filterTargetsKind, now it's (ts) is second.

What do you mean? The behavior is the same (note the backticks).

Uuh anyway this whole patch is not in a good shape right now. No need to review it until I polish it

Ah I got confused. Maybe (\kind -> filterTargetsKind kind ts) would be less mind-twisting, or adding filterTargetKinds :: [ComponentKind] -> ...

fgaz · 2017-11-09T17:42:51Z

This is now ready for review.

The failing test is obsolete, I'm ~~updating~~removing it right now.

23Skidoo · 2017-11-12T00:15:41Z

CI failures look legit.

fgaz · 2017-11-12T10:14:33Z

Yeah, I probably have to update/delete that test too since I changed new-run's behavior

23Skidoo · 2017-11-22T08:25:38Z

CI is still red, there's a failing test.

fgaz · 2017-12-19T11:51:20Z

With this set of changes that test should have that result. I tried to change it to check the new result, but there is little/no documentation about IntegrationTests2. Maybe I should just delete it

phadej · 2017-12-19T21:46:46Z

travis fails because we need for #4962 to get in.

phadej

Sorry for the late review. Few nitpicks.

I'd really like to see a positive test.

phadej · 2017-12-19T21:57:29Z

cabal-install/Distribution/Client/CmdRun.hs

-           selectComponentTargetBasic pkgid cname subtarget t
-  | otherwise
-  = Left (TargetProblemComponentNotExe pkgid cname)
+  = case availableTargetComponentName t


amend the haddock of selectComponentTarget.

We should be precise here when we mean "executable" and when "executable-like" component. This will cause confusion otherwise.

phadej · 2017-12-19T21:58:47Z

cabal-install/Distribution/Client/CmdRun.hs

-     ++ "contains just one executable.\n\n"
+     ++ "Any executable/test/benchmark in any package in the project can be "
+     ++ "specified. A package can be specified if contains just one "
+     ++ "executable. The default is to use the package in the current "


executable-like component

phadej · 2017-12-19T22:03:07Z

cabal-install/tests/IntegrationTests2.hs

@@ -869,15 +869,6 @@ testTargetProblemsRun config reportSubCase = do
      [ ( CmdRun.TargetProblemNoTargets, mkTargetPackage "p-0.1" )
      ]

-    reportSubCase "test-only"


I'm not super happy of simply removing this test.

It's invalid with this change, as https://github.com/haskell/cabal/blob/fcabd87406effc0c265312eb38509804682e4623/cabal-install/tests/IntegrationTests2/targets/test-only/p.cabal has only test-suite stanza, but now has an executable.

Let's change that package to be lib-only (you cannot run that).

What is pity, we don't have any positive tests for CmdRun in IntegrationsTests2.hs. We really should add them, so the things working now continue working in the future.

Done. Except the positive test. I'll do that in another pr

fgaz · 2018-01-09T14:39:13Z

I'm merging this once it's green, since the 15th is near.

I promise to do that positive test sometime after my next exam (which is also too near)

This is especially useful for passing arguments to a single test suite Closes haskell#4684.

Report possible tests and benchmarks too when erroring because of multiple available targets.

Since now new-run can run tests too (but obviously not libs)

As it can now mean a proper executable, a test, or a benchmark

The old comment was probably copypasted from CmdBuild. Now it reflects what new-run actually does.

fgaz · 2018-01-17T22:44:17Z

Oh well, windows and linux is green and one of the four osx builds is green too (the other 3 keep erroring). I'm merging this.

phadej · 2018-01-17T22:45:45Z

🎆

fgaz added cabal-install: nix-local-build type: enhancement labels Nov 3, 2017

phadej reviewed Nov 3, 2017

View reviewed changes

23Skidoo added the blocked: action-required label Nov 3, 2017

fgaz force-pushed the new-run/allow-running-tests-benchs branch from 4bea5cf to 40cf289 Compare November 9, 2017 16:19

fgaz removed the blocked: action-required label Nov 9, 2017

fgaz changed the title ~~Allow new-run to run tests and benchmarks too [WIP]~~ Allow new-run to run tests and benchmarks too Nov 9, 2017

fgaz requested a review from phadej November 9, 2017 17:43

fgaz force-pushed the new-run/allow-running-tests-benchs branch from fc255a3 to 5e92bcf Compare November 10, 2017 10:49

fgaz mentioned this pull request Nov 12, 2017

Replace flag-defaulting hack by proper solution #4886

Merged

1 task

fgaz force-pushed the new-run/allow-running-tests-benchs branch from 828fa50 to eefdbfe Compare November 12, 2017 19:45

23Skidoo added the blocked: action-required label Nov 22, 2017

fgaz force-pushed the new-run/allow-running-tests-benchs branch from eefdbfe to b1a70fe Compare December 19, 2017 11:01

phadej previously requested changes Dec 19, 2017

View reviewed changes

fgaz mentioned this pull request Jan 6, 2018

new-run should cd to the package directory before running the exe #5001

Closed

fgaz force-pushed the new-run/allow-running-tests-benchs branch 2 times, most recently from 9462c25 to e731514 Compare January 9, 2018 14:29

fgaz removed the blocked: action-required label Jan 9, 2018

fgaz added 4 commits January 9, 2018 17:40

Allow new-run to run tests and benchmarks too

a80c158

This is especially useful for passing arguments to a single test suite Closes haskell#4684.

Mention new-running tests/benchs in the changelog

74a7477

Docs: new-run test/bench is now allowed

9378f02

Better new-run error reporting

51bf62c

Report possible tests and benchmarks too when erroring because of multiple available targets.

fgaz added 3 commits January 9, 2018 17:40

Switch the failing test from test- to lib-only

d653795

Since now new-run can run tests too (but obviously not libs)

Use 'executable-like' in new-run documentation

caba6e7

As it can now mean a proper executable, a test, or a benchmark

Fix an obsolete comment

0ef5bb0

The old comment was probably copypasted from CmdBuild. Now it reflects what new-run actually does.

fgaz force-pushed the new-run/allow-running-tests-benchs branch from e731514 to 0ef5bb0 Compare January 9, 2018 16:40

Remove now invalid test

1454b2f

fgaz mentioned this pull request Jan 14, 2018

Add positive test for new-run #5032

Open

2 tasks

fgaz merged commit 67276f8 into haskell:master Jan 17, 2018

fgaz mentioned this pull request Jan 17, 2018

Passing arguments to test suites run via new-test is unsupported #4643

Closed

fgaz mentioned this pull request Aug 19, 2022

cabal test sees the executable, but cabal run does not. #8391

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow new-run to run tests and benchmarks too #4861

Allow new-run to run tests and benchmarks too #4861

fgaz commented Nov 3, 2017 •

edited

Loading

phadej Nov 3, 2017 •

edited

Loading

fgaz Nov 3, 2017

phadej Nov 3, 2017

fgaz commented Nov 9, 2017 •

edited

Loading

23Skidoo commented Nov 12, 2017

fgaz commented Nov 12, 2017

23Skidoo commented Nov 22, 2017

fgaz commented Dec 19, 2017

phadej commented Dec 19, 2017

phadej left a comment

phadej Dec 19, 2017

phadej Dec 19, 2017

phadej Dec 19, 2017

fgaz commented Jan 9, 2018

fgaz commented Jan 17, 2018

phadej commented Jan 17, 2018

Allow new-run to run tests and benchmarks too #4861

Allow new-run to run tests and benchmarks too #4861

Conversation

fgaz commented Nov 3, 2017 • edited Loading

phadej Nov 3, 2017 • edited Loading

Choose a reason for hiding this comment

fgaz Nov 3, 2017

Choose a reason for hiding this comment

phadej Nov 3, 2017

Choose a reason for hiding this comment

fgaz commented Nov 9, 2017 • edited Loading

23Skidoo commented Nov 12, 2017

fgaz commented Nov 12, 2017

23Skidoo commented Nov 22, 2017

fgaz commented Dec 19, 2017

phadej commented Dec 19, 2017

phadej left a comment

Choose a reason for hiding this comment

phadej Dec 19, 2017

Choose a reason for hiding this comment

phadej Dec 19, 2017

Choose a reason for hiding this comment

phadej Dec 19, 2017

Choose a reason for hiding this comment

fgaz commented Jan 9, 2018

fgaz commented Jan 17, 2018

phadej commented Jan 17, 2018

fgaz commented Nov 3, 2017 •

edited

Loading

phadej Nov 3, 2017 •

edited

Loading

fgaz commented Nov 9, 2017 •

edited

Loading