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

gradle build now passes out-of-the-box on a windows machine with java1.8 #31

Merged
merged 5 commits into from Mar 24, 2015
Merged

gradle build now passes out-of-the-box on a windows machine with java1.8 #31

merged 5 commits into from Mar 24, 2015

Conversation

gbranchaudrubenovitch
Copy link
Contributor

Made the minimal fixes so that gradle build completes successfully on my windows 8.1 machine with jvm 1.8.40 installed.

The fixes were mostly simple:

  • javadoc fixes
  • unicode-escape the utf-8 chars (the windows console is not utf-8)
  • change the expected content-length of a gzipped-response (my assumption is that this is due to changes to the implementation of the gzipstream classes in jdk1.8, but it could be due to Windows specificity too, such as new line characters)

I am open to any and all feedback.
Thanks!

…e JVM in a windows terminal is spawned with a default charset of CP-1252, rendering utf-8 characters in a stringunreadable. Encoding them solved this issue.
…a JVM1.6/Linux and a JVM1.8/Windows platform.
@gbranchaudrubenovitch gbranchaudrubenovitch changed the title gradle build now passes out-of-the-box on a windows machine with java1.8 gradle build now passes out-of-the-box on a windows machine with java1.8 Mar 23, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.61% when pulling a521bfa on gbranchaud:master into 5cda712 on testinfected:master.

@gbranchaudrubenovitch
Copy link
Contributor Author

I inlined the utf-8 chars.

Interesting findings regarding the compression test...

with response = request.header("Accept-Encoding", "gzip; q=0.9, deflate").get("/");, I get a stream that begins with 6d c2 c2 on my Windows 8.1 pc/JDK 1.8.40.

with response = request.header("Accept-Encoding", "gzip").get("/");, I get a stream that begins with 1f c2 8, but of course the Content-Encoding header check fails (it expects deflate but receives gzip).

None of those match the gzip magic number of 1f 8b 08.

According to the internet, deflate should not result in a magic number. The fact that the magic number is present on your environment is something I cannot explain...

I tried this alternative approach:

@Test
public void compressingResponses() throws IOException {
    response = request.header("Accept-Encoding", "gzip; q=0.9, deflate").get("/");
    assertThat(response).isOK()
                        // We expect deflate compression, which is preferred by the client
                        .hasHeader("Content-Encoding", "deflate");

    DeflaterInputStream deflateStream = new DeflaterInputStream(new ByteArrayInputStream(response.body()));
    Assert.assertThat(deflateStream.read(), not(equalTo(-1)));
}

This passes, but I am not sure if this more solid or if a DeflaterInputStream will just decode anything good or bad...

If you have any other idea, I am open.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.61% when pulling 01d2096 on gbranchaud:master into 5cda712 on testinfected:master.

@testinfected
Copy link
Owner

Sorry my bad, the complete test would be:

String GZIP_HEADER = new String(new char[]{0x1f, 0x8b, 0x08});

@Test
public void compressingResponses() throws IOException {
    response = request.header("Accept-Encoding", "deflate; q=0.9, gzip").get("/");
    assertThat(response).isOK()
        // We expect gzip compression, which is preferred by the client
        .hasHeader("Content-Encoding", "gzip")
        .hasBodyText(startsWith(GZIP_HEADER));
}

Note that the client prefers gzip in this case.

Can you try that?

According to the gzip format, the compressed stream is supposed to start with magic number 1f 8b followed by a byte describing the compression method, which I assume will be deflate (0x08).

I'm really surprised you're getting magic number 1f c2. Can you double check?

@gbranchaudrubenovitch
Copy link
Contributor Author

Ok, your test passes! I am not sure what I did to get in my previous confusing state, but your test works and is clean.

I committed & pushed it.

Thanks a lot for the support :)

testinfected added a commit that referenced this pull request Mar 24, 2015
gradle build now passes out-of-the-box on a windows machine with java1.8
@testinfected testinfected merged commit 6a6111b into testinfected:master Mar 24, 2015
@testinfected
Copy link
Owner

Thanks Gab, love the changes!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.61% when pulling 96d8eb2 on gbranchaud:master into 5cda712 on testinfected:master.

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.

None yet

3 participants