This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 846049 - DriftFileSenderTest.doNotSendEmptyZipFile fails on java7
DriftFileSenderTest.doNotSendEmptyZipFile fails on java7
Status: CLOSED NOTABUG
Product: JBoss Operations Network
Classification: JBoss
Component: Plugin Container (Show other bugs)
JON 3.1.1
Unspecified Unspecified
high Severity unspecified
: ---
: JON 3.1.1
Assigned To: John Sanda
Mike Foley
:
Depends On: 838681
Blocks: 682878
  Show dependency treegraph
 
Reported: 2012-08-06 12:42 EDT by John Sanda
Modified: 2012-08-31 14:36 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 838681
Environment:
Last Closed: 2012-08-31 14:36:28 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description John Sanda 2012-08-06 12:42:49 EDT
+++ 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@redhat.com 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@redhat.com 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 14:05:54 EDT
Fixes/changes have been pushed to the release/jon3.1.x branch.

commit hash: e24bbc6a0acf
Comment 2 John Sanda 2012-08-13 22:16:56 EDT
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 14:36:28 EDT
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.