Skip to content

BSP fixes #5698

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

Merged
merged 15 commits into from
Aug 20, 2025
Merged

BSP fixes #5698

merged 15 commits into from
Aug 20, 2025

Conversation

alexarchambault
Copy link
Collaborator

This fixes several small BSP issues, and tweaks the BSP snapshot tests too.

In particular, this makes the BSP snapshot tests more exhaustive, by testing a meta-build too, and making some tests more broad (main class request test).

This fixes issues with the buildTargetInverseSources request:

  • it makes return the correct target when passed a build.mill file (while it doesn't return anything without the changes here)
  • it ensures a compile reporter is around when evaluating tasks for that request, which is required if a sources task depends on a compile one for example (as a check requires such a reporter to be around during all task evaluations in the BSP server, so that we don't miss any diagnostic)

This PR commits can be reviewed one-by-one, and provide a bit of explanation of what each commit does in the commit message.

There's no reason not to use it any more, now that --bsp-install works
fine with a broken build too
The changed match is actually exhaustive. The changes here make that
explicit.
Keep module instance around, so that we don't check the bspModulesById
map twice, which limits the odds of discrepancies (in case of refactoring, …)
That request can be checked for all targets, not just a subset of them.
So that we have non-regression tests for it too
…ources

Passing a build.mill file to buildTargetInverseSources used not to return
anything. This changes that.
If a sources task depends on a compile one, a BSP-specific compile
reporter needs to be around in that request too. This fixes that.
@alexarchambault
Copy link
Collaborator Author

@lihaoyi It seems UTEST_UPDATE_GOLDEN_TESTS=true writes a truncated value when we give it a string with too many lines maybe. See how it truncates integration/ide/bsp-server/resources/snapshots/workspace-build-targets.json and adds ... at the end of it here.

@lihaoyi
Copy link
Member

lihaoyi commented Aug 14, 2025

@alexarchambault could you try updating this line upstream and adding height=99999 to see if that solves the problem? If that fixes it can send a PR to utest and i'll cut a new release

@lihaoyi
Copy link
Member

lihaoyi commented Aug 14, 2025

def goldenLiteralPrinter(x: Any): String = pprint.PPrinter.BlackWhite.apply(x).plainText

@lihaoyi
Copy link
Member

lihaoyi commented Aug 14, 2025

@lefou
Copy link
Member

lefou commented Aug 15, 2025

Changes look good to me.

lihaoyi pushed a commit to com-lihaoyi/utest that referenced this pull request Aug 18, 2025
This makes golden testing not truncate test data when it's too large.

See discussion in com-lihaoyi/mill#5698
@lihaoyi
Copy link
Member

lihaoyi commented Aug 18, 2025

@alexarchambault uTest 0.9.1 should have your fix

If a sources task depends on a compile one, a BSP-specific compile
reporter needs to be around in that request too. This fixes that.
@alexarchambault alexarchambault marked this pull request as ready for review August 19, 2025 20:36
 Conflicts:
	integration/ide/bsp-server/resources/snapshots/logging
@alexarchambault alexarchambault merged commit 8d20431 into com-lihaoyi:main Aug 20, 2025
62 of 63 checks passed
@alexarchambault alexarchambault deleted the bsp-fixes branch August 20, 2025 13:33
@lefou lefou added this to the 1.0.4 milestone Aug 20, 2025
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.

3 participants