-
Notifications
You must be signed in to change notification settings - Fork 306
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
Set go DebugConfguration working directory for tests #6748
Conversation
Hi, @tpasternak! I'm checking in to ask if you're going to take a look at this.
This point from the checklist is really confusing |
Sorry, I'm only partially available this week.cc @agluszak |
@@ -452,6 +453,21 @@ private static ExecutableInfo parseScriptPathFile( | |||
workspaceName, | |||
args.get(1)) | |||
.toFile(); | |||
|
|||
Matcher testTarget = TEST_TARGET.matcher(text); | |||
if (testTarget.find()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that thanks to this if
this shouldn't break anything
@@ -409,6 +409,8 @@ private static File findExecutable(Label target, List<File> outputs) { | |||
private static final Pattern TEST_SRCDIR = Pattern.compile("TEST_SRCDIR=([^ ]+)"); | |||
// Matches RUNFILES_<NAME>=<value> | |||
private static final Pattern RUNFILES_VAR = Pattern.compile("RUNFILES_([A-Z_]+)=([^ ]+)"); | |||
// Matches TEST_TARGET=//<package_path>:<target> | |||
private static final Pattern TEST_TARGET = Pattern.compile("TEST_TARGET=//([^:]*):([^\\s]+)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested it both with bzlmod and without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have bzlmod in some form in our company's repo which I have also tested this on, however I'm not familiar with what it does and what implications it can bring. So, no, and I don't really know how to test this extensively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the main difference between bzlmod and non-bzlmod projects is the introduction of canonical repository names. Also some rulesets behave differently depending on whether bzlmod is enabled or not (however user-visible differences are usually a bug).
I was asking because whenever there's a target pattern with a single @
or without any @
s it is possible that under bzlmod it got turned into a @@
(we - JetBrains - have run into such problems in out Hirschgarten repo).
But if it works with bzlmod it's much more probable that it will work without it than the other way round :)
So I'll merge it if it works for you!
This commit breaks runfiles discovery in tests when debugging as it changes the working directory to the runfiles tree, but doesn't set |
I sent #6883 to fix the regression. |
Fixes #2213
Checklist
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.
Discussion thread for this change
Issue number: #2213
Description of this change
I have ran into the same issue and wanted to fix it. I don't know if it is desirable for someone to have a different working directory for "run" and "debug" actions and I'm not familiar enough with the project to make a feature flag for this.
I have tested multiple project layouts to figure out the working directory used for bazel test and the "run" action for the ide. The mattern always seems to match the
<path_to_workspace>/<package_path>
which I went for and which fixed the issues for me.<path_to_workspace>
is the directory with the BUILD file where the corresponding target is defined