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.

Sign up for GitHub

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

Jump to bottom

In windows ScriptingContainerTest fails #2964

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

In windows ScriptingContainerTest fails #2964

cshupp1 opened this issue May 20, 2015 · 3 comments
Labels
tests windows
Milestone
JRuby 9.0.0.0

Comments

@cshupp1
Copy link

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
Copy link
Author

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 as completed 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
@cngshow
This is a fix for bug ID 2964. The URL to see the write up is here:
6556c49 
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
@gpbowman-git gpbowman-git mentioned this issue May 22, 2015
Merged
@headius
Copy link
Member

headius commented Jul 7, 2015

Thanks for your help!

@headius headius closed this as completed Jul 7, 2015
@headius headius added this to the JRuby 9.0.0.0.rc2 milestone Jul 7, 2015
@enebo
Copy link
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
Assignees
No one assigned
Labels
tests windows
Projects
None yet
Milestone
JRuby 9.0.0.0
Development

No branches or pull requests
3 participants
@headius @enebo @cshupp1