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

In windows ScriptingContainerTest fails #2964

Closed
cshupp1 opened this Issue May 20, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@cshupp1

cshupp1 commented May 20, 2015

In windows the test testGetCurrentDirectory in ScriptingContainer fails as follows:

org.junit.ComparisonFailure: expected:<C:[\Users\Cris\IdeaProjects\jruby\]core> but was:<C:[/Users/Cris/IdeaProjects/jruby/]core>
        at org.junit.Assert.assertEquals(Assert.java:115)
        at org.junit.Assert.assertEquals(Assert.java:144)
        at org.jruby.embed.ScriptingContainerTest.testGetCurrentDirectory(ScriptingContainerTest.java:1914)

The bug appears to be here (org.jruby.util.JRubyFile):

    public static String normalizeSeps(String path) {
        if (Platform.IS_WINDOWS) {
            return path.replace(File.separatorChar, '/');
        } else {
            return path;
        }

The order of replace is backwards, changing to:

   public static String normalizeSeps(String path) {
        if (Platform.IS_WINDOWS) {
            return path.replace('/', File.separatorChar);
        } else {
            return path;
        }
    }

With your permission my coding partner and I would like to use this simple issue to attempt our first contribution to JRuby. Is there a check in guide we should read?

Thanks,

Cris and Greg

@cshupp1

This comment has been minimized.

Show comment
Hide comment
@cshupp1

cshupp1 May 20, 2015

Sadly, our proposed solution leads to a different test failing, namely the 'testRequire' test in file 'org.jruby.test.TestKernel'.

We have a proposed fix for that too. Consider the original method:

    public void testRequire() throws Exception {
        //reset the $loadTestvar
        eval("$loadTest = nil");
        assertEquals("failed to load the file test/loadTest", "0", eval("require '../test/loadTest'"));
        assertEquals("incorrectly reloaded the file test/loadTest", "", eval("require '../test/loadTest'"));

        assertEquals("incorrect value for $\" variable", "true", eval("print $\"[-1].end_with?('test/loadTest.rb')"));
    }

Changing to:

    public void testRequire() throws Exception {
        char s = File.separatorChar;
        //reset the $loadTestvar
        eval("$loadTest = nil");
        assertEquals("failed to load the file test/loadTest", "0", eval("require '../test/loadTest'"));
        assertEquals("incorrectly reloaded the file test/loadTest", "", eval("require '../test/loadTest'"));
        assertEquals("incorrect value for $\" variable", "true", eval("print $\"[-1].end_with?('test" + s + "loadTest.rb')"));
    }

yields this in windows:

Results :

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

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] JRuby ............................................. SUCCESS [0.463s]
[INFO] JRuby Core ........................................ SUCCESS [2:42.917s]
[INFO] JRuby Truffle ..................................... SUCCESS [0.290s]
[INFO] JRuby Lib Setup ................................... SUCCESS [8.875s]
[INFO] JRuby Integration Tests ........................... SUCCESS [2.135s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2:55.031s
[INFO] Finished at: Wed May 20 17:16:33 EDT 2015
[INFO] Final Memory: 40M/463M
[INFO] ------------------------------------------------------------------------

Hurrah!

cshupp1 commented May 20, 2015

Sadly, our proposed solution leads to a different test failing, namely the 'testRequire' test in file 'org.jruby.test.TestKernel'.

We have a proposed fix for that too. Consider the original method:

    public void testRequire() throws Exception {
        //reset the $loadTestvar
        eval("$loadTest = nil");
        assertEquals("failed to load the file test/loadTest", "0", eval("require '../test/loadTest'"));
        assertEquals("incorrectly reloaded the file test/loadTest", "", eval("require '../test/loadTest'"));

        assertEquals("incorrect value for $\" variable", "true", eval("print $\"[-1].end_with?('test/loadTest.rb')"));
    }

Changing to:

    public void testRequire() throws Exception {
        char s = File.separatorChar;
        //reset the $loadTestvar
        eval("$loadTest = nil");
        assertEquals("failed to load the file test/loadTest", "0", eval("require '../test/loadTest'"));
        assertEquals("incorrectly reloaded the file test/loadTest", "", eval("require '../test/loadTest'"));
        assertEquals("incorrect value for $\" variable", "true", eval("print $\"[-1].end_with?('test" + s + "loadTest.rb')"));
    }

yields this in windows:

Results :

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

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] JRuby ............................................. SUCCESS [0.463s]
[INFO] JRuby Core ........................................ SUCCESS [2:42.917s]
[INFO] JRuby Truffle ..................................... SUCCESS [0.290s]
[INFO] JRuby Lib Setup ................................... SUCCESS [8.875s]
[INFO] JRuby Integration Tests ........................... SUCCESS [2.135s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2:55.031s
[INFO] Finished at: Wed May 20 17:16:33 EDT 2015
[INFO] Final Memory: 40M/463M
[INFO] ------------------------------------------------------------------------

Hurrah!

@cshupp1 cshupp1 closed this May 20, 2015

@cshupp1 cshupp1 reopened this May 20, 2015

gpbowman-git pushed a commit to gpbowman-git/jruby that referenced this issue May 22, 2015

This is a fix for bug ID 2964. The URL to see the write up is here:
jruby#2964

On windows the mvn -Ptest fails due to incorrect substitution of '/' and '\'.

 See the write up for more details.

 Cris Shupp
 Greg Bowman
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jul 7, 2015

Member

Thanks for your help!

Member

headius commented Jul 7, 2015

Thanks for your help!

@headius headius closed this Jul 7, 2015

@headius headius added this to the JRuby 9.0.0.0.rc2 milestone Jul 7, 2015

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Jul 8, 2015

Member

I reverted this. It made bundler stop working (surprised revert did not pop up here). I am going to suggest the right fix is to change this test. I think the main meaning of it is to see if we have the same logic File and not exact same string. So:

        String expResult = System.getProperty("user.dir");
        String result = instance.getCurrentDirectory();
        assertEquals(expResult, result);

Maybe should wrap both values as java.io.File() and do some equality check that way.

Member

enebo commented Jul 8, 2015

I reverted this. It made bundler stop working (surprised revert did not pop up here). I am going to suggest the right fix is to change this test. I think the main meaning of it is to see if we have the same logic File and not exact same string. So:

        String expResult = System.getProperty("user.dir");
        String result = instance.getCurrentDirectory();
        assertEquals(expResult, result);

Maybe should wrap both values as java.io.File() and do some equality check that way.

@enebo enebo modified the milestones: JRuby 9.0.0.0, JRuby 9.0.0.0.rc2 Jul 8, 2015

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