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

feat: add support for test runner with scala-test #29

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

imclem
Copy link
Contributor

@imclem imclem commented Oct 27, 2024

Hi there 👋

I'm not sure if it's the appropriate way to do it, but I wanted to run the tests from within ZED and got it working using the query in this commit. (Here's a video showing it in action: https://www.loom.com/share/f3c286309e064774a7a8872b5b68a99e)

This is my tasks.json

[
  {
    "label": "run tests with bloop",
    "name": "run-bloop-test",
    "command": "./run-bloop-test.sh ${ZED_WORKTREE_ROOT} ${ZED_FILE}",
    "working_directory": "${workspace_root}",
    "environment": {
      "PATH": "${env.PATH}"
    },
    "keystroke": "cmd-shift-r",
    "tags": ["scala-test"]
  }
]

And the run-bloop-test.sh script:
I've haven't added guards or proper opt parsing or anything, I just wanted to hack my way around running the tests from zed:

#!/bin/bash

project_name=$(basename $1)
filename=$(basename $2 | sed 's/\.scala//')
bloop test $project_name -o "*$filename*"

@imclem imclem changed the title feat: add support for test gutter with scala-test feat: add support for test runner with scala-test Oct 27, 2024
type: (type_identifier) @run
)
) @_scala_test_class_end
(#match? @run "^(AnyWordSpec|WordSpec|AnyFunSpec|FunSpec|AnyFunSuite|FunSuite|AnyFlatSpec|FlatSpec|FeatureSpec|AnyFeatureSpec|AnyPropSpec|PropSpec|AnyFreeSpec|FreeSpec)$")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is ZED using code lenses at all? This way seems it might not work in all the adge cases, would be better to leave resolving test classes to Metals as it has the knowledge about what test suites are available.

People might name something FreeSpec which might not actually be a test

Copy link
Contributor Author

@imclem imclem Oct 28, 2024

Choose a reason for hiding this comment

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

Is ZED using code lenses at all?

I don't know.

This way seems it might not work in all the adge cases, would be better to leave resolving test classes to Metals as it has the knowledge about what test suites are available.

I don't know how to do that for zed, I took inspiration from the runnables provided by zed:
https://github.com/zed-industries/zed/blob/main/crates/languages/src/go/runnables.scm

People might name something FreeSpec which might not actually be a test

Agreed, it's actually true for the existing queries for App in the existing runnables.scm:
Screenshot 2024-10-28 at 18 16 15

I tried messing around and also matching an import for org.scalatest.wordspec.AnyWordSpec without success.

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 find a mention of code lenses in Zed docs, so this might no be possible to use. That's the standard way of adding such additional information.

Is it possible to invoke something before showing the runnables or that is purely syntactic? Maybe it would be possible to query code lense and then show them as runnables?

Although looking at what you linked this seems standard to Zed. The go runnables also depend on regex and discovering things named correctly. That seems like a pretty bad idea overall, I wonder what is their explanation for doing that 🤔

I guess until code lenses are ready we could go with this workaround 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Though this does depend on Bloop available. Are we able to have this turned on conditionally? So that users can define the task themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to invoke something before showing the runnables or that is purely syntactic? Maybe it would be possible to query code lense and then show them as runnables?

I don't think it's possible, you just provide a runnables.scm and if a task is available with the same tag the run button is displayed.

Though this does depend on Bloop available. Are we able to have this turned on conditionally? So that users can define the task themselves?

The code in the PR does not depend on bloop at all, the task and the script have been provided for testing purposes.

By default, (it's the same for the main runner) no run icon is displayed, in order to have a run icon displayed, as a zed user, you must add a task with the same tag as the one in the runnables.scm. In this particular case, scala-test.

IMHO, the plugin should not decide how the tests are to be run, but it would be a good idea to provide an example in the documentation. It took me some time to understand how to get the Run Icon for tests and main methods...

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Let's merge it then. If you're interested we could add some more docs in here or in Metals under Zed (I don't think we have anything yet)

@tgodzik tgodzik merged commit 247aa84 into scalameta:main Oct 29, 2024
@imclem
Copy link
Contributor Author

imclem commented Oct 29, 2024

Let's merge it then. If you're interested we could add some more docs in here or in Metals under Zed (I don't think we have anything yet)

@tgodzik I'll gladly add some documentation, please let me know where ? In the README.md in this project and in metals ?

@tgodzik
Copy link
Contributor

tgodzik commented Oct 29, 2024

You can add it in the README

Or if you feel like it we can add a page for Zer in https://github.com/scalameta/metals/tree/main/docs/editors

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.

2 participants