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

Set which encoding your test JVM will start with #1735

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

awilkins
Copy link

This patch adds a setting to the java.test.config object.

{
  "java.test.config": {
    "encoding": "ISO-8859-1",
  },
}

The aim here is to be able to seamlessly test code which cares about the launch encoding of the JVM. Yes, such code is not good. But sometimes you don't control your dependencies enough to fix this.

Since the debug launcher already cares about the encoding member of the config, the only thing required is to add the member, which is missing from the launch configs generated by the test plugin.

// In `configurationProvider.ts:` (vscode-java-debug)
// VS Code internal console uses UTF-8 to display output by default.
if (config.console === "internalConsole" && !config.encoding) {
    config.encoding = "UTF-8";
}

Fixes #1641

NB : I couldn't work out how to actually launch and gather results from Java tests, in the Typescript test suite, but I did add a Java test case that cares about the launch encoding and tested it manually.

This patch adds a setting to the `java.test.config` object.

```jsonc
{
  "java.test.config": {
    "encoding": "ISO-8859-1",
  },
}
```
The aim here is to be able to seamlessly test code which cares
about the launch encoding of the JVM. Yes, such code is not good.
But sometimes you don't control your dependencies enough to fix
this.

Since the debug launcher already cares about the `encoding` member of the
config, the only thing required is to add the member, which is missing
from the launch configs generated by the test plugin.

```typescript
// In `configurationProvider.ts:` (vscode-java-debug)
// VS Code internal console uses UTF-8 to display output by default.
if (config.console === "internalConsole" && !config.encoding) {
    config.encoding = "UTF-8";
}
```

Fixes microsoft#1641
@awilkins
Copy link
Author

awilkins commented Sep 24, 2024

PS : had to change the update site for eclipse to

            <repository location="https://download.eclipse.org/eclipse/updates/4.33/R-4.33-202409030240/"/>

: the older URL doesn't seem available any more. I'm presuming your builds are behind a Maven repo that has this stuff cached, but building the source as-is from publicly available builds isn't currently possible.

I've not submitted this as part of the PR (because I figure you have that Maven repo), but I'm mentioning it so you know.

@jdneo
Copy link
Member

jdneo commented Sep 25, 2024

PS : had to change the update site for eclipse to

            <repository location="https://download.eclipse.org/eclipse/updates/4.33/R-4.33-202409030240/"/>

: the older URL doesn't seem available any more. I'm presuming your builds are behind a Maven repo that has this stuff cached, but building the source as-is from publicly available builds isn't currently possible.

I've not submitted this as part of the PR (because I figure you have that Maven repo), but I'm mentioning it so you know.

Updated the target platform definition in #1734

@jdneo
Copy link
Member

jdneo commented Oct 8, 2024

Hi @awilkins,

Sorry that I'm just back from vacation and I will review this ASAP.

@jdneo jdneo self-requested a review October 8, 2024 00:25
@jdneo jdneo added this to the 0.43.0 milestone Oct 9, 2024
@@ -254,6 +254,11 @@
"markdownDescription": "%configuration.java.test.config.javaExec.description%",
"default": ""
},
"encoding": {
Copy link
Member

Choose a reason for hiding this comment

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

Since java.test.config can either be an array or an object. So we need to duplicated this under item section as well. (start from line 349)

Copy link
Author

Choose a reason for hiding this comment

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

Also noted, because they're right next to each other, that it's vmargs in the array and vmArgs in the singular ; have changed the case of the array to match (this has been deprecated 4 years, I think it might have been long enough :-) )

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

Successfully merging this pull request may close these issues.

"vmArgs": ["-Dfile.encoding=iso885915"] seems to be overided by something else in the java.test.config config
2 participants