Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Ide test integration #46

Closed

Conversation

santiweight
Copy link

@santiweight santiweight commented Nov 22, 2022

This is a proposal for integrating various testing frameworks' output format so that they can be used in IDEs that support Haskell. There is a design for two APIs for test frameworks: getTestTree which outlines the test tree of tests in a test suite, and runTest which allows for a test group or test case to be run from the test tree.

Feedback is very helpful, especially from testing framework maintainers, so that I can better understand what use cases are most important and we don't miss anything important.

Rendered

@santiweight santiweight changed the title Ide test integration RFC: Ide test integration Nov 22, 2022
Copy link
Contributor

@david-christiansen david-christiansen left a comment

Choose a reason for hiding this comment

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

I added some general feedback - hope it's useful.

[.NET with VSC](https://marketplace.visualstudio.com/items?itemName=formulahendry.dotnet-test-explorer)
and many others.

Often these IDEs will supply inline hints for "Run this test" and
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, I think a screenshot would be very useful for Haskellers who don't use IntelliJ much. This one came up from a quick image search, for instance: https://i.stack.imgur.com/j5f1o.png

expectations from a mature language.

Haskell has a mature and well-maintained testing ecosystem, including
the libraries: `tasty`, `HUnit`, and `hspec`. While these libraries
Copy link
Contributor

Choose a reason for hiding this comment

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

What about QuickCheck and friends? Is the intention that they are subsumed by the mention of Tasty here?

the libraries: `tasty`, `HUnit`, and `hspec`. While these libraries
themselves are mature, there is currently minimal
integration between Haskell's testing libraries and HLS, as well as
the primary IDEs used for Haskell (VSC;Emacs;Neovim).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the primary IDEs used for Haskell (VSC;Emacs;Neovim).
the primary IDEs used for Haskell (VSC, Emacs, and Neovim).

2. [vim-test](https://github.com/vim-test/vim-test)

While these are valuable contributions to the ecosystem, their choice
of regex for the collection of the user's test tree, and support for only
Copy link
Contributor

Choose a reason for hiding this comment

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

The term "test tree" is not necessarily understood by the audience for this proposal . Based on our discussion elsewhere, I think you made a good observation about how Haskell test libraries are typically (but not always) organized, and how it's different from something like JUnit-derived tests. In particular, JUnit-inspired libraries typically use the language's notion of a declaration or definition to bound tests, so a test is a class or a method, while Haskell libraries more frequently use an ordinary data structure to represent tests. This means that we'd need to be a bit creative about the UI, and draw inspiration from things like IntelliJ while still doing something a bit different that fits Haskell better.

I think the proposal would be accessible to more readers if these assumptions and observations were made more explicit in this background section.

Haskell's major testing frameworks, starting with `tasty`.
3. Integrate test tree output output with VSC's Haskell extension, via VSC's [Testing API](https://code.visualstudio.com/api/extension-guides/testing).
4. Stretch goal: support IDEs other than VSC.
5. Stretch goal: support test results caching, and features such as "Rerun only failed tests".
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't tell if this goal is a subgoal of number 3, or if it's something that should be supported by the other frameworks. Can you clarify it?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is, is it a part of the JSON output format, or is it a VS Code extension feature?

maintain the integration library.
3. The author cannot see how to acquire the test tree that is common to
IDE test integration, since it requires inspecting the typechecking artifacts
of all files in the test suite.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a true limitation with respect to your proposal. Haskell test libraries really do organize things on a basis other than top-level declarations, and it would be good to not reinvent the wheel. I like that your proposal is highly compatible with what everyone's already using.

of all files in the test suite.
4. Sometimes testing frameworks will provide conflicting `IsTest` instances for the
the same type, which is a problem when using multiple testing frameworks in the
same codebase.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the permanent problem of type classes. Such conflicting instances would always be orphans, right? I think this is no worse than any other Haskell type class.

-- `SrcLocs` are used to indicate where inline hints should be displayed
-- in an IDE.
data TestTree =
TestGroup {name :: Text, subTrees :: [TestTree], location :: SrcLoc}
Copy link
Contributor

Choose a reason for hiding this comment

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

From where should the testing library get the SrcLoc? It's not easy for Haskell programs to view their own source code like this, and I'd be worried to promote an ecosystem-wide convention that requires fragile kludges to be used successfully.

Choose a reason for hiding this comment

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

I think we definitely want the location to be optional.

needs until after integration is alright out in the real world.

Here are some features that have been neglected in this proposal:
1. Distinction between "error" and "failure" when a test fails.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not a mere matter of adding an additional constructor to TestResult?

3. Support for options when running a test, such as: `--accept` when
running golden tests, `-jN` to run tests in parallel, and options
related to property-based tests.
4. Support for localized options when running a test.
Copy link
Contributor

Choose a reason for hiding this comment

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

What are localized options? In my head it would be something like replacing --accept with --godtage to make the Danish localization, but this doesn't seem like a very plausible interpretation of what you're writing.

@david-christiansen
Copy link
Contributor

One more question: how do we expect the IDE to get from the current filename to the name of the test suite executable to be run? I know how HLS could do this, but the proposal is written to be independent of HLS as far as I can see.

Comment on lines +34 to +35
Attempts at integration between testing frameworks have focused on
regex searches, with support for hspec:

Choose a reason for hiding this comment

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

neotest uses https://tree-sitter.github.io/tree-sitter/ with https://github.com/tree-sitter/tree-sitter-haskell instead of regex. It still has its shortcomings to detect test cases, but at least in terms of syntax recognition it's much more robust as it uses a real parser.

Comment on lines +51 to +52
1. Ability for IDEs with Haskell support to provide inline hints for "Run
Tests" style commands.

Choose a reason for hiding this comment

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

In visual studio code and other editors supporting the language server protocol, this is often accomplished with code-lens, in combination with custom client-side commands.
This is something that afaik the go and rust language-servers use and support.

Another example where they use the language-server but without code-lens would be https://github.com/microsoft/vscode-java-test
It uses the executeCommand message with a custom command implementation that returns test locations (https://github.com/microsoft/vscode-java-test/blob/main/java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/handler/TestDelegateCommandHandler.java)

Comment on lines +49 to +71
Support for IDE integration of testing libraries will provide the following
benefits:
1. Ability for IDEs with Haskell support to provide inline hints for "Run
Tests" style commands.
2. Ability for IDEs with Haskell support and support for a test explorer
view to display the user's test tree and run individual tests.
3. A unified framework for testing libraries to provide integration with
IDEs. When a new testing framework is developed, they will be able to
provide the results in a Haskell datatype via serialization, which
avoids framework-specific test tree discovery logic.
4. Potential to share test result rendering logic between different testing
frameworks, whereas currently each testing framework has their own test result
display logic.

## Goals

1. Create a new library, `hs-test`, with common datatypes for testing framework
test trees, and test results.
2. Generate test trees and test results that can be serialized into JSON from
Haskell's major testing frameworks, starting with `tasty`.
3. Integrate test tree output output with VSC's Haskell extension, via VSC's [Testing API](https://code.visualstudio.com/api/extension-guides/testing).
4. Stretch goal: support IDEs other than VSC.
5. Stretch goal: support test results caching, and features such as "Rerun only failed tests".

Choose a reason for hiding this comment

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

As far as I am aware, this kind of functionality is often implemented on the client side/test-extension itself and doesn't need extra per language implementation.

Comment on lines +67 to +68
2. Generate test trees and test results that can be serialized into JSON from
Haskell's major testing frameworks, starting with `tasty`.

Choose a reason for hiding this comment

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

There would also be the Build Server Protocol which is adressing these requirements, but so far it didn't see much adaption so I guess JSON is be the safer/easier route. But maybe it's still useful to have a look at it and see how they modelled their solution.

Copy link

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

I do think this proposal would be more likely to get adoption if it was more abstract

Tests" style commands.
2. Ability for IDEs with Haskell support and support for a test explorer
view to display the user's test tree and run individual tests.
3. A unified framework for testing libraries to provide integration with

Choose a reason for hiding this comment

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

We should be able to offer a unified CLI interface also, perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a really nice result!

IDEs. When a new testing framework is developed, they will be able to
provide the results in a Haskell datatype via serialization, which
avoids framework-specific test tree discovery logic.
4. Potential to share test result rendering logic between different testing

Choose a reason for hiding this comment

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

Up until this point I thought this was mostly about test discovery, but now it sounds like you also want to standardise test reporting. That might be harder!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that running individual tests from the editor requires both. Discovery allows something like HLS to say "this is a test, want to run it?", and reporting allows HLS to say "It ran, and it passed".


1. Create a new library, `hs-test`, with common datatypes for testing framework
test trees, and test results.
2. Generate test trees and test results that can be serialized into JSON from

Choose a reason for hiding this comment

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

I think you have in mind concepts of "test tree" and "test results", it would really help if you made that explicit. The technical section below does that, but I think it would be helpful to have something in the initial prose, and maybe an argument that it's adequate for all usecases!

| TestCase {name :: Text, location :: SrcLoc}

data RunTestsRequest =
RunTestsRequest {path :: NonEmpty Text}

Choose a reason for hiding this comment

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

Working out what I imagine would happen here:

  • The path gets turned into a tasty pattern
  • The pattern matches multiple tests
  • Multiple tests get run

Seems somewhat bad but maybe acceptable?

-- `SrcLocs` are used to indicate where inline hints should be displayed
-- in an IDE.
data TestTree =
TestGroup {name :: Text, subTrees :: [TestTree], location :: SrcLoc}

Choose a reason for hiding this comment

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

I think we're almost certainly going to want this to be extensible in some way. I think it's very likely that different testing frameworks will want to annotate the tree with framework-specific metadata of their own, and we should make supporting that a key design goal.

-- `SrcLocs` are used to indicate where inline hints should be displayed
-- in an IDE.
data TestTree =
TestGroup {name :: Text, subTrees :: [TestTree], location :: SrcLoc}

Choose a reason for hiding this comment

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

I think we definitely want the location to be optional.

timeTaken :: Maybe Double
}

data TestResult = Success | Skipped | Failure FailureReason

Choose a reason for hiding this comment

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

This is why I think this really needs to be extensible!

- find the test tree
- execute a test run request, and display the results to the user.

One problem that could occur is that having the IDE run `cabal test` to

Choose a reason for hiding this comment

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

I think having to actually run a cabal command to find this out is a weakness. Junit and friends can know most of this information statically, just by compiling the code, which is quite nice! That does mean that they have to rely on stuff like the hierarchy being implicit in static information (like the class hierarchy), but it is otherwise quite nice. I think we should at least think about whether we can do this more statically (or at least without cabal in the loop!!).

Choose a reason for hiding this comment

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

e.g. HLS has some plugins that load code into the interpreter and run it, so we could plausibly compute the test tree without going through cabal, if we do need to run something.


I have chosen the option in this proposal for the following reasons:
1. Such an approach would run Haskell code in GHCi, which is slower, and
would therefore scale poorly for production codebases.

Choose a reason for hiding this comment

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

I also don't understand this. For running the tests you could still run them through cabal or whatever, assuming that you have some way of knowing how to do that.

@michaelpj
Copy link

michaelpj commented Nov 29, 2022

I do think this proposal would be more likely to get adoption if it was more abstract. Sketching some ideas:

class TestSpecification t where
    type TestResult :: *
    isSuccess :: TestResult t -> Bool
    details :: TestResult t -> Text
    
    type TestKey :: *
    enumerateTests :: t -> [TestKey t]
    runTest :: TestKey t -> IO (TestResult t)

class (TestSpecification t, TestKey t ~ NonEmpty Fragment) => HierarchicalTestSpecification t where
    type Fragment :: *
   
type TestReport t = Map (TestKey t) (TestResult t)

produceTestReport :: (TestSpecification t) => t -> IO (TestReport t)
produceTestReport t = fromList <$> for (enumerateTests t) $ \key -> do
    report <- runTest key
    pure (key, report)

This is 5 minutes of thinking, but I think this would give people a lot of flexibility.

@gbaz
Copy link
Collaborator

gbaz commented Dec 17, 2022

In our last TWG meeting we talked about this a bit. I think there's generally favorable sentiment. One big sticking point that came us is that while mainly "produce a test tree" is a mode existing test frameworks are good at, annotating it with source locations is not. There's an idea that HasCallStack constraints can be used to extend existing frameworks for this in a lightweight way, which would help resolve that. That said, I'd still be onboard with making such locations optional but encouraged.

@santiweight
Copy link
Author

In our last TWG meeting we talked about this a bit. I think there's generally favorable sentiment.

Thanks for the response. I am still interested in following up on this, but was ill for the last two weeks.

I think the optional but encouraged nature of the test tree is exactly what we should push for. I do think that producing these locations is not particularly difficult with the HasCallStack.

Thankfully, we can still push for this just with the "test tree" on its own, since that is probably the best way to handle Haskell tests. And it seems clear that we can add optional SrcLoc annotations. So I think I will continue down this road.

I do think this proposal would be more likely to get adoption if it was more abstract.

I understand this sentiment, and I will consider how to be abstract.

There is a problem with abstractness however, is that a the test-plugin-for-ides level, we suddenly need a single joining point, where all the abstractness becomes concrete so we can display results etc.

I'm happy to go with something abstract, but that, for the time being, every TestResult t must be translatable to some PluginTestResult. Unfortunately, this removes a lot of the usefulness of a flexible TestResult. I'm not sure that maintainers are going to be inclined to use the flexible TestResult for uses other than the plugin, and the plugin requires something less abstract.

What use case would something abstract provide if not integration with a plugin?

@david-christiansen
Copy link
Contributor

The question about abstraction was intended for @michaelpj , right?

@michaelpj
Copy link

One big sticking point that came us is that while mainly "produce a test tree" is a mode existing test frameworks are good at, annotating it with source locations is not.

Perhaps worth pointing out in the proposal that there are two ways of slicing this problem:

  1. Run the test harness to discover tests, use this information to locate the tests in the source
  2. Interrogate the source to discover tests, use this information to work out how to run them

The characteristic of approach 2 is using some kind of source annotation to identify tests. Approach 2 has the advantage that you don't need to complicate the runtime with source information. AFAICT, approach 2 is much more common in other languages: Java, Rust, Python etc., all have test frameworks that work primarily off source annotations. That's not to say we should do it, but perhaps worth being clear why not. (And we could do it: Haskell has annotations!)

There is a problem with abstractness however, is that a the test-plugin-for-ides level, we suddenly need a single joining point, where all the abstractness becomes concrete so we can display results etc.

Okay, so what does abstractness get us?

  1. Richer test harness behaviour

We are passing this information across a serialization boundary, because we are running the discovery in one process, recording the results, and then feeding that back into the harness. If we serialize to JSON, then in one sense the real interface is that a TestTree is a Value! But this is much less informative than saying that a TestTree is some type specific to the test harness which can be converted to and from Value. This is nicer for the implementor to begin with, but also since the JSON is being produced and consumed by the same code they can pass through additional information easily:

MyTestCase
--> discover
JSON
--> run
MyTestCase

So test harnesses can assume they have their own richer TestTree type all the time, which is nice.

  1. Flexibility

Having an abstract interface that just specifies exactly what we need makes it easier for other people to fit their own system into it. For example, it was only when sketching out an abstract version that I realised that it might be nice to just have a "flat" enumerateTests that would work just fine for test frameworks that don't arrange their tests hierarchically. We can then optionally enrich it with additional things if we have a hierarchical arrangement.

The risk, of course, is that it's over-engineered and nobody ever uses this additional power 🤷

I'm happy to go with something abstract, but that, for the time being, every TestResult t must be translatable to some PluginTestResult.

Right, we need to be able to interpret the test results generically, that's true. This is a rare time when I find myself wishing for subtyping :D


Having written these things, I'm not feeling confident that I'm saying sensible things. I think I'd want to actually try it out before having more opinions 😅

@santiweight
Copy link
Author

Thanks for the response @michaelpj. I neglected to add that discussion about why Haskell is different, which was had in this discourse comment. You can also find in various points in that thread discussion around using Haskell's annotation system, which was shot down pretty hard.

Right, we need to be able to interpret the test results generically, that's true. This is a rare time when I find myself wishing for subtyping :D

I think this is why I want to put off your concerns of abstractions for follow up to this proposal. A majority of the benefit here is found in simply being able to run Haskell tests in VSC. And we should make sure to design something that is extensible to an abstract design without throwing away implementation work.

All this said: I get the feeling that an implementation is what's needed after this discussion. I will see if I can find the time over this Christmas to get an MVP working :)

@gbaz
Copy link
Collaborator

gbaz commented Dec 19, 2022

@santiweight be sure to check in with @davean who worked up a proof-of-concept of the HasCallStack approach that you could leverage.

@david-christiansen
Copy link
Contributor

@santiweight How are things going with this proposal? Is there something we can do to get it unstuck?

@santiweight
Copy link
Author

Hey thanks for checking in. Just finished a three week holiday today. I don't think there's anything blocking me right now.

@gbaz How might I check in with @davean? I don't know what project you're referring to.

My goal for the future is getting an MVP plugin for VSC. I don't think it will be too hard, and I'll surely take a lot of shortcuts.

Some overall thoughts:

  1. I am personally happy with the source location annotations using HasCallStack and will be using that for MVP
  2. I will make a very monomorphic data structure and target exclusively tasty test suites
  3. I don't expect an MVP to be too challenging, and mostly about learning VSC's typescript plugins

From the current discussion, I want to implement and iterate. I get the sense that trying to corral Haskell's ecosystem ahead of time is premature (but I expect we will want to do some corralling down the line...).

@gbaz
Copy link
Collaborator

gbaz commented Jan 13, 2023

I think david will send an email connecting you. What he has is basically a proof of concept, and if you already have a HasCallStack proof of concept that works equally well, you may not get much out of his -- at the time I suggested you two connect, it was not clear to me how much (if any) progress you had made in that regard. I look forward to seeing what you come up with!

@santiweight
Copy link
Author

santiweight commented Jan 23, 2023

I've made some good progress. Current support is for running tests with a patched version of tasty that produces a JSON object of available tests. The tests are subsequently run via command line with a filter matching the requested test.

Screen.Recording.2023-01-22.at.10.50.03.PM.mov

@santiweight
Copy link
Author

santiweight commented Jan 23, 2023

There is a small wrinkle that I'm not sure we can overcome without enabling deep subsumption.

In order to have a location attached to the correct source location, we need to have a HasCallStack in scope for the declaration containing a test definition.

In order to achieve this, we could define:

data TestTreeInner = TestCase | TestGroup
type TestTree = HasCallStack => TestTreeInner

instead of the current:

data TestTree = TestCase | TestGroup

It's not ideal imo. I'm open to suggestions here.


@gbaz @davean btw I never received an email afaik.

@santiweight
Copy link
Author

santiweight commented Jan 23, 2023

I also have locations in the text working...

Screen.Recording.2023-01-22.at.11.00.39.PM.mov

@michaelpj
Copy link

Nothing specific to say except that looks super cool!

@santiweight
Copy link
Author

santiweight commented Jan 26, 2023

Help needed and welcome!

There are a few tasks left to do that I don't know how to resolve.

  1. I need to find a way to get the available test projects in a VSC workspace. Subsequently I need to add better caching file-watching based on the files in a test project. Is this what hie-bios is for?
  2. Preferably we can find a way to avoid the need for a HasCallStack constraint on the function that calls testCase or testGroup. Above I suggested a solution that requires DeepSubsumption which is not satisfactory imo...

Any input on either of these? If not then I'll just go and put out a crappy first try version.

@fendor
Copy link

fendor commented Jan 27, 2023

Without having read the full context (sorry, just saw your question re hie-bios), hie-bios's main purpose is to find the compilation option given a filepath or some other way to identify a part of your project. Any caching it does is for performance reasons.
HLS caches these components, and should know all the files it has loaded. What caching are you referring to in particular? (We can discuss it a bit more on libera)

@santiweight
Copy link
Author

santiweight commented Jan 27, 2023

Thanks @fendor. I misspoke in my comment. I meant "file-watching" - so that I know when the user's test tree becomes stale. I'm not sure the best way to find either what test projects are available in a folder (they might not be at the top level of the folder), nor do I know how to do file-watching for a cabal project.

I'm assuming both of these use cases have been paved?

@fendor
Copy link

fendor commented Jan 27, 2023

I'm not sure the best way to find either what test projects are available in a folder

For a cabal project, I think there is no built-in way to do it, unless something like haskell/cabal#7500 lands.
You can find the components via a query such as cat dist-newstyle/cache/plan.json | jq '."install-plan"|.[]|select(.style=="local")|select(."component-name" != null) | select(."component-name" | startswith("test:"))|."component-name"', which basically just queries plan.json for any local component whose component-name starts with test:. (or using cabal-plan directly)

nor do I know how to do file-watching for a cabal project

Personally, also not sure. I have some ideas, but I don't know of a way to do it right now.

@santiweight
Copy link
Author

I tried to come back to this because I think it's really cool, but I don't know the way forward with respect to file watching.

Does anyone have any clue who we could reach out to about this?

It would be a shame to have this die, because it does work, and we're not terribly far away from an MVP.

@david-christiansen
Copy link
Contributor

Well, I seem to have missed this final question - sorry. For file watching, I'd post the question on Discourse - I don't know a more specific forum to check.

I'm going to temporarily close this one - please feel free to reopen it if you want to keep the discussion going and make progress!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants