Bug 846049 - DriftFileSenderTest.doNotSendEmptyZipFile fails on java7
Summary: DriftFileSenderTest.doNotSendEmptyZipFile fails on java7
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: JBoss Operations Network
Classification: JBoss
Component: Plugin Container
Version: JON 3.1.1
Hardware: Unspecified
OS: Unspecified
high
unspecified
Target Milestone: ---
: JON 3.1.1
Assignee: John Sanda
QA Contact: Mike Foley
URL:
Whiteboard:
Depends On: 838681
Blocks: 682878
TreeView+ depends on / blocked
 
Reported: 2012-08-06 16:42 UTC by John Sanda
Modified: 2012-08-31 18:36 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of: 838681
Environment:
Last Closed: 2012-08-31 18:36:28 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description John Sanda 2012-08-06 16:42:49 UTC
+++ This bug was initially created as a clone of Bug #838681 +++

[17:47:26] <pilhuhn> jshaughn hey, I am looking at org.rhq.core.pc.drift.DriftFilesSenderTest#doNotSendEmptyZipFileToServer because this is failing for me
[17:48:14] <pilhuhn> now when I step into the code, I see the DritftFilesSender.run() using the DriftClientTestStub
[17:48:24] <pilhuhn> which in org.rhq.core.pc.drift.DriftClientTestStub#sendChangeSetContentToServer
[17:48:37] <pilhuhn> unconditionally increases the counter, which then gets checked in the test at
[17:48:51] <pilhuhn> assertEquals(driftClient.getSendChangeSetContentInvocationCount(), 0
[17:49:03] <pilhuhn> no it is pretty old, but I am looking into jdk7 and mvn 3
[17:54:10] <jshaughn> pilhuhn: so this is passing on Java6?


[19:53:10] <jshaughn> Heiko, ok, so there is a good reason why this test passes on Java6.  It's because Java6 throws an exception when trying to close a ZipOutputStream that has had nothing added to it.  This happens the line *before* driftClient.sendChangeSetContentToServer(), it's caught and handled with logging.
[19:56:34] <jshaughn> Given the lack of comments it almost seems like it was not expected by John but maybe it was.  Now, it seems that Java7 maybe allows the empty zip.  Looking for some doco on this
[20:10:28] <jshaughn> well I can't find anything saying this was explicitly changed although I wonder if maybe it's fallout from the new Zip FileSystem support
[20:12:09] <jshaughn> Anyway, I think this may require an actual code change if in fact J7 has started allowing empty zips

[21:02:51] <jshaughn> right, I haven't run locally with J7 but it seems that perhaps they now allow an empty zip
[21:03:09] <pilhuhn> Let me open a BZ for this
[21:03:20] <jshaughn> If you can call ZipOutputStream.close() and not get an exception then we have a problem
[21:04:21] <jshaughn> because that will in fact send the empty zip to the server, which, apparently, we don't want to do.  Although, I'm not sure why we prevent it.
[21:05:15] <jshaughn> I guess it's just an optimization because it would be a waste of time, I guess.  And perhaps there is server code that would not behave well
[21:05:51] <jshaughn> So, we will need to, I guess, not depend on the exception

--- Additional comment from hrupp on 2012-07-17 18:14:12 EDT ---

There is clearly a difference in java 6 and 7

This fails on 6 and works on 7:


    @Test
    public void testEmptyZip() throws Exception {
        File file = new File("/tmp/my.zip");
        if (file.exists())
            file.delete();

        FileOutputStream fos = new FileOutputStream(file);
        ZipOutputStream zos = new ZipOutputStream(fos);
        zos.finish();
        zos.close();

    }

Did not yet find a bug reference though.

--- Additional comment from ccrouch on 2012-08-02 15:33:54 EDT ---

Assigning to jsanda since it seems he was the original author of the drift related code.

Comment 1 John Sanda 2012-08-06 18:05:54 UTC
Fixes/changes have been pushed to the release/jon3.1.x branch.

commit hash: e24bbc6a0acf

Comment 2 John Sanda 2012-08-14 02:16:56 UTC
Moving to ON_QA since JON 3.1.1 ER2 build is availble - https://brewweb.devel.redhat.com/buildinfo?buildID=228250

Comment 3 Mike Foley 2012-08-31 18:36:28 UTC
java7 removed as a feature from jon 3.1.1.  java7 is not a requirement, ergo there can be no bug based on this.


Note You need to log in before you can comment on or make changes to this bug.