Skip to content

Conversation

@jjalowie
Copy link
Contributor

@jjalowie jjalowie commented Oct 3, 2024

(Work in progress)

This is a draft of adding LIT test discovery in the VSCode MLIR extension.
The implemented functionality adds a list of LIT tests in the Testing Tab in VSCode which looks like this:
image

Things I want to implement:

  • LIT test discovery (the test folder path can be configured in .vscode/settings.json)
  • in case of LIT test failures the failure log is displayed for the programmer (just using the lit.py [...] -v flag)
  • passing arguments to lit.py which can be configured through .vscode/settings.json
  • running a single as well as a subset of LIT tests through the "Run tests" button
  • (not sure if this is feasible, I'd be most interested in this feature, @River707 could you hint me how to approach this?) listing all test chunks when using the --split-input-file option and enabling triggering a single chunk (test case) within the LIT test instead of running the whole file; implementation of this feature depends on whether each chunk has some sort of unique ID within mlir-opt and the triggered chunks could be called by it (similiar to how --gtest_filter works in gtest, e.g. imagine something like --mlir-opt_filter test.mlir*_42 gets implemented which would just chunk no. 42 from test.mlir).
  • (optionally) debugging LIT tests, i.e. running the mlir-opt command of the LIT tests in a configurable debugger (lldb/gdb)

@jjalowie
Copy link
Contributor Author

jjalowie commented Oct 3, 2024

@River707 could you have a look? do you have any thoughts about the proposed changes?

@River707
Copy link
Contributor

River707 commented Oct 4, 2024

Thanks @jjalowie, definitely a really nice thing to add! It's something I've thought about a little bit from time to time. I'm really happy you're picking this up, would love to see this kind of feature get added.

Before commenting on the PR itself, here are some general thoughts/feedback/requirements I have on the feature in general:

  • Build system agnostic
    Whatever support we add should work just as well with bazel as it does cmake, and most any other build system. Anchoring on lit is a pretty good thing here, because it should help avoid over-fitting for one specific build flow.

  • Has to work well for non-upstream build projects
    I've got a fairly strong view that whatever tools we add for the extension need to work outside of the LLVM monorepo. I'd say most usages of MLIR fit this view, so it's important that we keep that in mind (in can impact how you think about structuring things).

  • Minimal user setup or configuration
    We should try to avoid adding configs as much as possible, and try to make the initial setup as easy as possible. Nothing is nicer than when something "just works" after getting it set up. For the testing, let's make sure that we infer as much as possible when we can.

Now for some feedback directed towards the PR itself:

  • lit.py config
    Does this need to be a required config? In many cases users already have lit in their path, so they wouldn't need to set a config, would be good to not require this. We could also emit a warning or something if we can't find lit (so they can set it manually)

  • test_root_folder config
    Do we even need this config at all? That question has a few parts:

    • Many projects will often have multiple test directories, so you'd need to support multiple from the beginning (but that can be kind of an annoying thing to need to provide)
    • Could we somewhat infer this by searching the workspace for directories containing lit.site.cfg.py? (the file that gets generated containing all of the replaced config variables). That'd probably give you the majority of what you need. In a situation where lit is in the user path, this would mean users could have tests picked up and run automatically (with no config tweaking).
  • Discovering tests
    Right now you have a recursive file scanner for this, but you could likely infer this by calling into lit with --show-tests --show-suites. That should give you things like the source root directory, the tests included, etc. This would be a lot more accurate because lit knows how to respect things like excludes/etc. which can impact what is considered a test.

  • listing all test chunks when using the --split-input-file option and enabling triggering a single chunk
    This one is kind of hard because lit doesn't differentiate chunks as different tests, so we'd be kind of fudging it. There are potentially complex ways that I think could do this, but I think this is best left for discussion after getting a really good base experience down (that is already a huge win).

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