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

Improve error message in case of reading of old dump file #319

Merged
merged 1 commit into from Oct 2, 2015

Conversation

Projects
None yet
4 participants
@marchof
Member

marchof commented Oct 1, 2015

Speaking on behalf of @benzonico, who maintains JaCoCo integration with SonarQube:

"java.io.IOException: Incompatible version 1007."
is not user-friendly.

Indeed, this message is hard for understanding by end-users, while it can reach them during attempt to read old dump file. See for example discussion http://stackoverflow.com/questions/30459260/jacoco-sonarqube-incompatible-version-1007

@Godin Godin added this to the 0.7.6 milestone May 28, 2015

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof May 28, 2015

Member

@Godin Any proposal what would be more user-friendly? Some hint for resolution, e.g.

Execution data was written in a different version (1007). Please use same JaCoCo version for writing and reading execution data

Member

marchof commented May 28, 2015

@Godin Any proposal what would be more user-friendly? Some hint for resolution, e.g.

Execution data was written in a different version (1007). Please use same JaCoCo version for writing and reading execution data

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Jun 3, 2015

Member

@Godin I also wonder whether JaCoCo should introduce a specific IOException (IncompatibleExecFileVersionException extends IOException) so Sonar could react in a more user friendly way?

The general question is whether todays exec file format is the correct exchange format in the long run. Possible alternatives are:

  1. Use XML report format which means builds need to produce those
  2. Make exec files self describing by e.g. adding probe positions within methods, so the analyzer does not depend on the probe insertion strategy
Member

marchof commented Jun 3, 2015

@Godin I also wonder whether JaCoCo should introduce a specific IOException (IncompatibleExecFileVersionException extends IOException) so Sonar could react in a more user friendly way?

The general question is whether todays exec file format is the correct exchange format in the long run. Possible alternatives are:

  1. Use XML report format which means builds need to produce those
  2. Make exec files self describing by e.g. adding probe positions within methods, so the analyzer does not depend on the probe insertion strategy

somechris pushed a commit to somechris/jacoco that referenced this pull request Jun 28, 2015

Christian Aistleitner
Provide more detailed error message for format version mismatches
The error message

  Incompatible version 1007

was confusing, as it:
* Showed a hex string without a hex string marker,
* Showed an alienatingly high number, as it still contained the padding,
* Did not gave hints about what actually failed, why it failed, and
  how to get around it.

We now report incompatible versions like

  Failed to read version 6 data (This JaCoCo build can only read/write
  version 7 data). To read the version 6 data, use the same JaCoCo
  version that got used for writing it.

Also, we give JaCoCo users a chance to reliably identify version-
mismatch exceptions by providing a separate exception class for them.

Issue jacoco#319
@somechris

This comment has been minimized.

Show comment
Hide comment
@somechris

somechris Jun 28, 2015

This error message also alienated me, when coming across it while doing some
coding for facebook's buck (which is still at 'version 1006').

For me, there are three issues with this error message

  • It is not clear what the version number is.

    The number in the error message is a hex string, but without a hex
    string marker. So it's not apparent that it is a hex string. It is
    confusing to read "version 1007", but then seeing that the version
    number actually is 4103 (=0x1007). It makes me unsure how to refer
    to the version number (e.g.: in bug reports). Would I use 1007,
    0x1007, or 4103?

    For me, it would be less misleading to use plain decimals in error
    messages, as that's the format used by people in every day's life.

  • Ludicrously high version number.

    When encountering "version 1007", I first thought that version
    number reporting is broken. The number seemed too high to be
    plausible. On the one hand people are not accustomed to see such
    high version numbers. I certainly cannot relate to a number
    "1006". On the other hand, (since the numbers are closer to each
    other) the difference between "version 1006" and "version 1007"
    /looks/ less relevant, than the difference between "version 6" and
    "version 7". Since the version number was 0x1001 right from the
    start (See 2624878#diff-07e9e2777aabf57534238aea73df1c70R31
    ), it looks like the on-disk version number actually decomposes as

          0x1007         =     0x1000     +       7
       (on-disk version) = (some padding) + (real version number)
    

    The padding is fine on-disk. But when communicating with users, the
    padding only alienates by artificially showing a higher
    number. Hence, for me, it would be more user-friendly to only report
    the real version number. Thereby, we would be reporting version
    numbers like 6, 7, or 8 to users. These are version numbers people
    are used too.

  • No indication of a status quo, or a remedy.

    The error message did not contain any indication of where/how the
    version encountered, and how to remedy it.

    For me, it would be preferable to make clear that it's for the
    data file, and to show both the expected and actual version
    number.

Bottom line: I'd replace the error message

Incompatible version 1007

by

Failed to read version 6 data (This JaCoCo build can only read/write version 7 data). To read the version 6 data, use the same JaCoCo version that got used for writing it.

. I gave it a shot at #330 (along with the separate exception class brought forward by @marchof).

somechris commented Jun 28, 2015

This error message also alienated me, when coming across it while doing some
coding for facebook's buck (which is still at 'version 1006').

For me, there are three issues with this error message

  • It is not clear what the version number is.

    The number in the error message is a hex string, but without a hex
    string marker. So it's not apparent that it is a hex string. It is
    confusing to read "version 1007", but then seeing that the version
    number actually is 4103 (=0x1007). It makes me unsure how to refer
    to the version number (e.g.: in bug reports). Would I use 1007,
    0x1007, or 4103?

    For me, it would be less misleading to use plain decimals in error
    messages, as that's the format used by people in every day's life.

  • Ludicrously high version number.

    When encountering "version 1007", I first thought that version
    number reporting is broken. The number seemed too high to be
    plausible. On the one hand people are not accustomed to see such
    high version numbers. I certainly cannot relate to a number
    "1006". On the other hand, (since the numbers are closer to each
    other) the difference between "version 1006" and "version 1007"
    /looks/ less relevant, than the difference between "version 6" and
    "version 7". Since the version number was 0x1001 right from the
    start (See 2624878#diff-07e9e2777aabf57534238aea73df1c70R31
    ), it looks like the on-disk version number actually decomposes as

          0x1007         =     0x1000     +       7
       (on-disk version) = (some padding) + (real version number)
    

    The padding is fine on-disk. But when communicating with users, the
    padding only alienates by artificially showing a higher
    number. Hence, for me, it would be more user-friendly to only report
    the real version number. Thereby, we would be reporting version
    numbers like 6, 7, or 8 to users. These are version numbers people
    are used too.

  • No indication of a status quo, or a remedy.

    The error message did not contain any indication of where/how the
    version encountered, and how to remedy it.

    For me, it would be preferable to make clear that it's for the
    data file, and to show both the expected and actual version
    number.

Bottom line: I'd replace the error message

Incompatible version 1007

by

Failed to read version 6 data (This JaCoCo build can only read/write version 7 data). To read the version 6 data, use the same JaCoCo version that got used for writing it.

. I gave it a shot at #330 (along with the separate exception class brought forward by @marchof).

@laba69

This comment has been minimized.

Show comment
Hide comment
@laba69

laba69 Jul 4, 2015

Jacoco last version I installed :jacoco-0.7.6-20150603.214955-4
triggers the same error in eclipse:
Error while importing coverage session.
Reason: Error while reading execution data file : C:/tmp/jacoco.exec(code 5005)
in the details view I found: Incompatible version 1007.

I installed EclEmma jacoco plugin today.
This error is about the plugin or about my jacoco exec file?
How can i solve this properly? I'm not comfortable with maven.
Thank you for your contribution

laba69 commented Jul 4, 2015

Jacoco last version I installed :jacoco-0.7.6-20150603.214955-4
triggers the same error in eclipse:
Error while importing coverage session.
Reason: Error while reading execution data file : C:/tmp/jacoco.exec(code 5005)
in the details view I found: Incompatible version 1007.

I installed EclEmma jacoco plugin today.
This error is about the plugin or about my jacoco exec file?
How can i solve this properly? I'm not comfortable with maven.
Thank you for your contribution

GitHub #319: Improved error message for incompatible exec data files.
In case of incompatible execution data formats read from another JaCoCo
version ExecutionDataReader.read() now throws a
IncompatibleExecDataVersionException with a better error message.

marchof added a commit that referenced this pull request Oct 2, 2015

Merge pull request #319 from jacoco/issue-319
Improve error message in case of reading of old dump file.

@marchof marchof merged commit 46c167a into master Oct 2, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@marchof marchof deleted the issue-319 branch Oct 2, 2015

@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017

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