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

ConfigProviderTest TCK bug #664

Open
Joseph-Cass opened this issue Dec 3, 2020 · 4 comments
Open

ConfigProviderTest TCK bug #664

Joseph-Cass opened this issue Dec 3, 2020 · 4 comments
Labels
bug 🪲 An error in the implementation code or documentation
Milestone

Comments

@Joseph-Cass
Copy link

Describe the bug

@Test
public void testEnvironmentConfigSource() {
Assert.assertNotNull(config.getValue("path", String.class));
String value = System.getenv().get("MP_TCK_ENV_DUMMY");
Assert.assertNotNull(value);
Assert.assertEquals("dummy", value);
Assert.assertEquals(value, config.getValue("mp.tck.env.dummy", String.class));
}

testEnvironmentConfigSource() searches for a Config Property called "path". However, on Windows machines the environment variable "path" doesn't exist. The variable is called "Path".

This causes the Assert to fail.

@Joseph-Cass Joseph-Cass added the bug 🪲 An error in the implementation code or documentation label Dec 3, 2020
@radcortez
Copy link
Contributor

Interesting, I wonder how the TCK passes here: https://github.com/smallrye/smallrye-config/runs/1477581698

@Joseph-Cass
Copy link
Author

Hmm I'm not sure either. I was guessing that the Microsoft Windows Server 2019 server which the GithHub Actions uses may have a "path" or "PATH" environment variable, but I can't tell for sure...

@Emily-Jiang
Copy link
Member

@radcortez it depends on the Windows setting. Some Windows machine has PATH while others might have different variable name using different case. I think in the next release, we can improve the variable name mapping for env variable by ignoring the case as the fallback.

@Emily-Jiang
Copy link
Member

I took a further look at this. Since the test in particular is to test the environment added, I think we can remove the line that causes the test unable to function in some OS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 An error in the implementation code or documentation
Projects
None yet
Development

No branches or pull requests

3 participants