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

Fix issue with string replace methods removing backslashes #51

Merged
merged 1 commit into from
Sep 23, 2013
Merged

Fix issue with string replace methods removing backslashes #51

merged 1 commit into from
Sep 23, 2013

Conversation

NiXXeD
Copy link
Contributor

@NiXXeD NiXXeD commented Sep 23, 2013

Java string replace methods don't properly handle backslashes in the replacement string.
See http://stackoverflow.com/questions/9575116/forward-slash-in-java-regex

Suggestion as of that SO page is to replace backslash with double backslash ("" with "\") in the replacement string prior to replacing.

I ran into this because user.home on windows machines is something like C:\Users\NiXXeD, and several of the tests failed for me on a clean repo. I updated the affected tests to reproduce the issue prior to making changes.

…aths for instance as user.home, but other examples might exist).
@lviggiano
Copy link
Collaborator

nice catch. I never tried to build on windows. Thanks for this.

@NiXXeD
Copy link
Contributor Author

NiXXeD commented Sep 23, 2013

It's worth noting there are other issues building on windows, but I did not want to try and address those yet. I can poke around on it more tonight. There are about ~6 unit tests that fail for me. Unfortunately at work I'm on linux so I can't test here =)

lviggiano pushed a commit that referenced this pull request Sep 23, 2013
Fix issue with string replace methods removing backslashes
@lviggiano lviggiano merged commit 73c419e into matteobaccan:master Sep 23, 2013
@lviggiano
Copy link
Collaborator

Wow... 6 tests failing on Windows... I need to pay more attention to this.

lviggiano pushed a commit that referenced this pull request Sep 24, 2013
@lviggiano
Copy link
Collaborator

On Mon, Sep 23, 2013 at 10:58 PM, Andy Christianson <
notifications@github.com> wrote:

It's worth noting there are other issues building on windows, but I did
not want to try and address those yet. I can poke around on it more
tonight. There are about ~6 unit tests that fail for me. Unfortunately at
work I'm on linux so I can't test here =)

Hi Andy,

I think I fixed the problems related to Windows build.

I was not calling some OutputStream.close() on some test methods while
writing and overwriting properties files to test the reload() methods (and
hot reload too).

Now tests are green.

Thanks for pointing this out, so I could fix this.

I had to setup a VMWare virtual machine, setup an entire development
environment (git, maven, jdk) on the virtual machine and use my mac to
connect to the maven build via remote debug socket.

Thanks and best regards,

Luigi.

@NiXXeD
Copy link
Contributor Author

NiXXeD commented Sep 30, 2013

There's one last scenario (2 tests) that fail on windows. If the HOME environment variable isn't set (default for windows), two tests are failing to assert because null != "".

The tests are:
WithImportedPropertiesTest.testMultipleImports
WithImportedPropertiesTest.testSystemEnv

@lviggiano
Copy link
Collaborator

I just tried with latest master in my branch, and all tests are green:

C:\Development\src\owner>ver

Microsoft Windows XP [Version 5.1.2600]

C:\Development\src\owner>set
ALLUSERSPROFILE=C:\Documents and Settings\All Users
APPDATA=C:\Documents and Settings\Administrator\Application Data
CommonProgramFiles=C:\Program Files\Common Files
COMPUTERNAME=LUIGI-D7D9C9547
ComSpec=C:\WINDOWS\system32\cmd.exe
ERROR_CODE=0
FP_NO_HOST_CHECK=NO
HOME=C:\Documents and Settings\Administrator
HOMEDRIVE=C:
HOMEPATH=\Documents and Settings\Administrator
JAVA_HOME=C:\Development\Java\jdk1.7.0_40
LOGONSERVER=\\LUIGI-D7D9C9547
NUMBER_OF_PROCESSORS=1
OS=Windows_NT
Path=C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem
PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH
PROCESSOR_ARCHITECTURE=x86
PROCESSOR_IDENTIFIER=x86 Family 6 Model 23 Stepping 6, GenuineIntel
PROCESSOR_LEVEL=6
PROCESSOR_REVISION=1706
ProgramFiles=C:\Program Files
PROMPT=$P$G
SESSIONNAME=Console
SystemDrive=C:
SystemRoot=C:\WINDOWS
TEMP=C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp
TMP=C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp
USERDOMAIN=LUIGI-D7D9C9547
USERNAME=Administrator
USERPROFILE=C:\Documents and Settings\Administrator
windir=C:\WINDOWS

C:\Development\src\owner>\Development\Java\apache-maven-3.1.0\bin\mvn clean test
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building OWNER 1.0.5-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ owner ---
[INFO] Deleting C:\Development\src\owner\target
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ owner ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\Development\src\owner\src\main\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ owner ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 33 source files to C:\Development\src\owner\target\classes
[INFO]
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ owner ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 14 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ owner ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 66 source files to C:\Development\src\owner\target\test-classes
[WARNING] /C:/Development/src/owner/src/test/java/org/aeonbits/owner/examples/EncryptedPropertiesExample.java:[5,16] sun.misc.BASE64Decoder is internal proprietary API and may be removed in a future r
elease
[WARNING] /C:/Development/src/owner/src/test/java/org/aeonbits/owner/examples/EncryptedPropertiesExample.java:[28,43] sun.misc.BASE64Decoder is internal proprietary API and may be removed in a future
release
[INFO]
[INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ owner ---
[INFO] Surefire report directory: C:\Development\src\owner\target\surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.aeonbits.owner.ConfigFactoryTest
Tests run: 15, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.156 sec
Running org.aeonbits.owner.ConfigTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.313 sec
Running org.aeonbits.owner.ConfigURLFactoryTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.ConfigWithSubstitutionTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.debugging.ToStringTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.DisableFeatureTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.EqualsAndHashCodeTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.event.EventListenerOnReloadTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.141 sec
Running org.aeonbits.owner.event.EventListenerTest
Tests run: 23, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.156 sec
Running org.aeonbits.owner.event.UnmodifiablePropertiesTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.importedprops.ImportConfigTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.importedprops.SystemPropertiesAndEnvTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.importedprops.WithImportedPropertiesTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.interfaces.AccessibleConfigTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.078 sec
Running org.aeonbits.owner.interfaces.MutableConfigTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.032 sec
Running org.aeonbits.owner.loadstrategies.DefaultLoadStrategyTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.046 sec
Running org.aeonbits.owner.loadstrategies.FirstLoadStrategyTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.063 sec
Running org.aeonbits.owner.loadstrategies.MergeLoadStrategyTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec
Running org.aeonbits.owner.multithread.MultiThreadReloadTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.484 sec
Running org.aeonbits.owner.PropertiesInvocationHandlerTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.141 sec
Running org.aeonbits.owner.reload.AsyncAutoReloadTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.094 sec
Running org.aeonbits.owner.reload.HotReloadWhenURLContainsVariablesTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.047 sec
Running org.aeonbits.owner.reload.ReloadTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.047 sec
Running org.aeonbits.owner.reload.SyncAutoReloadTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.062 sec
Running org.aeonbits.owner.serializable.TestSerialization
{someText=someText, someArray=some,array}
{someText=someText, someArray=some,array}
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.078 sec
Running org.aeonbits.owner.StrSubstitutorTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.SystemVariableExpanderTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.062 sec
Running org.aeonbits.owner.typeconversion.arrays.ArraySupportTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.typeconversion.arrays.BasicArrayWithAnnotationTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.typeconversion.arrays.ConflictingAnnotationResolutionTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.typeconversion.arrays.ConflictingAnnotationsOnClassLevelTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec
Running org.aeonbits.owner.typeconversion.arrays.ConflictingAnnotationsOnMethodLevelTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec
Running org.aeonbits.owner.typeconversion.arrays.InvalidAnnotationTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.typeconversion.arrays.SeparatorAnnotationOnClassLevelTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec
Running org.aeonbits.owner.typeconversion.arrays.TokenizerAnnotationOnClassLevelTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.typeconversion.collections.CollectionSupportTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.typeconversion.ConverterClassTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec
Running org.aeonbits.owner.typeconversion.editor.PropertyEditorTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec
Running org.aeonbits.owner.typeconversion.PrimitiveTypesTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.032 sec
Running org.aeonbits.owner.typeconversion.SpecialTypesTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.062 sec
Running org.aeonbits.owner.UndefinedPropertiesTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec
Running org.aeonbits.owner.UnsupportedInstantiationTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec
Running org.aeonbits.owner.UtilTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.047 sec
Running org.aeonbits.owner.variableexpansion.PropertyNotSetTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.variableexpansion.VariableExpansionTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running org.aeonbits.owner.xml.XmlSourceTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.234 sec

Results :

Tests run: 224, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 9.312s
[INFO] Finished at: Mon Sep 30 17:54:42 CEST 2013
[INFO] Final Memory: 11M/37M
[INFO] ------------------------------------------------------------------------
C:\Development\src\owner>

What are you getting? Can you please update from my master, rerun the tests and let me know if things are still failing?

Thanks.

@NiXXeD
Copy link
Contributor Author

NiXXeD commented Sep 30, 2013

Yea, on my machine HOME env var isn't set.

I see in your log, yours is. When it's not set, that test is asserting the value it received (null) against a string which replaced the null with an empty string, so it fails.

When I mess with that variable, it also starts to mess with other things in windows (cygwin takes over that home too for instance). I can get my tests to pass by putting any value at all in HOME env var, but the real issue I think is that the test is reaching into global state which means they may not consistently pass.

The way I've typically handled sys props and env vars myself is to have a wrapper class around them that I can then mock appropriately in tests. That ensures that no system would be able to cause a failure.

@lviggiano
Copy link
Collaborator

You are right.

This is a fresh installation of Windows XP, but usually I use mingw as bash client. I don't know if that variable comes from there. I'll verify this. Thanks.

@lviggiano
Copy link
Collaborator

The way I've typically handled sys props and env vars myself is to have a wrapper class around them that I can then mock appropriately in tests. That ensures that no system would be able to cause a failure.

On master I did it already, I just need to set up the test properly.

@lviggiano
Copy link
Collaborator

Hi.

Just found that mvn 3.1.0 batch script sets the HOME variable at beginning

C:\Development\src\owner>type \Development\Java\apache-maven-3.1.0\bin\mvn.bat
@REM ----------------------------------------------------------------------------
@REM Licensed to the Apache Software Foundation (ASF) under one
@REM ...snip!
@REM ----------------------------------------------------------------------------

@REM Begin all REM lines with '@' in case MAVEN_BATCH_ECHO is 'on'
@echo off
@REM enable echoing my setting MAVEN_BATCH_ECHO to 'on'
@if "%MAVEN_BATCH_ECHO%" == "on"  echo %MAVEN_BATCH_ECHO%

@REM set %HOME% to equivalent of $HOME
if "%HOME%" == "" (set "HOME=%HOMEDRIVE%%HOMEPATH%")

Anyway, it's a good idea not to depend on it.

@lviggiano
Copy link
Collaborator

released version 1.0.5 few moments ago. The updated jar is already available in maven central repository. Have a look at the release notes. Thanks @NiXXeD for your support.

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