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

Error: Attempt to close tag classes when packages was the one open #384

Closed
nstepien opened this issue May 2, 2019 · 19 comments
Closed

Error: Attempt to close tag classes when packages was the one open #384

nstepien opened this issue May 2, 2019 · 19 comments

Comments

@nstepien
Copy link
Contributor

nstepien commented May 2, 2019

image

My jest tests have started reporting this error today, as it's failing to generate a valid coverage report.

The report xml file ends with just this:
image

I believe this started happening following the most recent update.

@nstepien
Copy link
Contributor Author

nstepien commented May 2, 2019

@coderica @coreyfarrell
It looks like df6e994 is the only commit that could've introduced this regression.
Please let me know if you need more information on my setup, it's a rather basic ts-jest config otherwise.

@coreyfarrell
Copy link
Member

@MayhemYDG I have reset latest istanbul-reports to 2.2.4. If you delete your package-lock.json or yarn.lock and reinstall you should get the prior version. It would be helpful if we could get a test added to istanbul-reports demonstrating this failure so we can protect against this happening again in the future. Please do not close this issue as it hasn't been fixed, just suppressed.

@coderica this happened as a result of #382. For now I have not reverted the patch from git but I have reverted the latest on npm to the version without your patch.

I'll do what I can to help resolve this situation but I have nearly zero knowledge of cobertura.

@nstepien
Copy link
Contributor Author

nstepien commented May 2, 2019

@coreyfarrell
image
It looks like @latest is still 2.2.5. I cleared my lockfile, my node_modules, even cleared the npm cache, and I'm still getting 2.2.5.

@coreyfarrell
Copy link
Member

@MayhemYDG that's not what I'm seeing from npm info istanbul-reports or from https://www.npmjs.com/package/istanbul-reports. I suspect npmjs has distributed servers and you are hitting a server that has not yet seen my change.

@nstepien
Copy link
Contributor Author

nstepien commented May 2, 2019

@coreyfarrell
image

Ah, you are correct, but I just figured out I'm still installing 2.2.5 because istanbul-api requires it.
679fc64#diff-69d6f06913bafe7128bf6ba176b2ecd2
It's the only package in my lockfile using it:

    "istanbul-api": {
      "version": "2.1.7",
      "resolved": "https://registry.npmjs.org/istanbul-api/-/istanbul-api-2.1.7.tgz",
      "integrity": "sha512-LYTOa2UrYFyJ/aSczZi/6lBykVMjCCvUmT64gOe+jPZFy4w6FYfPGqFT2IiQ2BxVHHDOvCD7qrIXb0EOh4uGWw==",
      "dev": true,
      "requires": {
        "async": "^2.6.2",
        "compare-versions": "^3.4.0",
        "fileset": "^2.0.3",
        "istanbul-lib-coverage": "^2.0.5",
        "istanbul-lib-hook": "^2.0.7",
        "istanbul-lib-instrument": "^3.3.0",
        "istanbul-lib-report": "^2.0.8",
        "istanbul-lib-source-maps": "^3.0.6",
        "istanbul-reports": "^2.2.5",
        "js-yaml": "^3.13.1",
        "make-dir": "^2.1.0",
        "minimatch": "^3.0.4",
        "once": "^1.4.0"
      }
    },

@coreyfarrell
Copy link
Member

Ahh sorry I missed that. I've just set istanbul-api@2.1.6 as latest. Could take a bit before your local npm servers see this change.

@nstepien
Copy link
Contributor Author

nstepien commented May 2, 2019

@coreyfarrell yup, it's all good now, thank you very much.

@coderica
Copy link
Contributor

coderica commented May 2, 2019

@MayhemYDG Sorry for the bug. I tested my change on my project but I guess I should have also tested on one that was already generating a properly formatted report.

Is there any way you can reproduce this in a test app I can use to investigate the issue?

@JaKXz
Copy link
Member

JaKXz commented May 2, 2019

It would be good to write some kind of snapshot test to make sure we can have the correct output produced in the cobertura reporter

@coderica
Copy link
Contributor

coderica commented May 2, 2019

yes, it sounds like we need a couple, for both this scenario and the one in #66

@coreyfarrell
Copy link
Member

@coderica I had to revert #382 via #385 to make sure the regression isn't accidentally released again. Moving forward I agree with @JaKXz, we need expanded testing with any changes to this report. I'm hoping you and @MayhemYDG can work together in some capacity to figure out a fix which serves both of your needs.

@nstepien
Copy link
Contributor Author

nstepien commented May 4, 2019

Okay here's a reduced test case:

package.json

{
  "scripts": {
    "test": "jest"
  },
  "devDependencies": {
    "istanbul-reports": "^2.2.5",
    "jest": "^24.7.1"
  },
  "jest": {
    "collectCoverage": true,
    "coverageReporters": [
      "cobertura"
    ]
  }
}

module.test.js

describe('reduced test case', () => {
  it('should generate a valid coverage report', () => {
    expect(false).toBe(true);
  })
});

Steps to reproduce:

  • npm i
  • npm t

You should then see something like

        Failed to write coverage reports:
        ERROR: Error: Attempt to close tag classes when packages was the one open
        STACK: Error: Attempt to close tag classes when packages was the one open
    at XMLWriter.closeTag (C:\istanbul\node_modules\istanbul-lib-report\lib\xml-writer.js:57:15)
    at CoberturaReport.onSummaryEnd (C:\istanbul\node_modules\istanbul-reports\lib\cobertura\index.js:73:14)
    at Visitor.<computed> [as onSummaryEnd] (C:\istanbul\node_modules\istanbul-lib-report\lib\tree.js:34:30)
    at ReportNode.Node.visit (C:\istanbul\node_modules\istanbul-lib-report\lib\tree.js:122:17)
    at Tree.visit (C:\istanbul\node_modules\istanbul-lib-report\lib\tree.js:150:20)
    at C:\istanbul\node_modules\istanbul-api\lib\reporter.js:94:18
    at Array.forEach (<anonymous>)
    at Reporter.write (C:\istanbul\node_modules\istanbul-api\lib\reporter.js:92:35)
    at C:\istanbul\node_modules\@jest\reporters\build\coverage_reporter.js:235:18
    at Generator.next (<anonymous>)

If you don't specify istanbul-reports version 2.2.5 in package.json, the coverage report won't fail.
If you make the test pass, the coverage report won't fail, although I've seen it fail will all tests passing before.

@bcoe
Copy link
Member

bcoe commented May 6, 2019

@coderica nothing to be sorry about, the problem was lack of test coverage on our part (ironically).

None of the core Istanbul maintainers use cobertura, so we're not the best in the world at landing patches for it. Excited to work with you to re-land the patch 👍

@KevinGrandon
Copy link

Any chance at publishing istanbul-api@2.1.8 either with a revert or a fix? Starting to see many folks have failures with cobertura coverage reporting failures.

@coreyfarrell
Copy link
Member

I've republished istanbul-reports@2.2.4 as 2.2.6 so this should no longer be an issue. You may have to refresh your package-lock.json or yarn.lock.

@jesussobrino
Copy link

I've republished istanbul-reports@2.2.4 as 2.2.6 so this should no longer be an issue. You may have to refresh your package-lock.json or yarn.lock.

I've just tried, and I still have the same issue.

@coreyfarrell
Copy link
Member

I've just tried, and I still have the same issue.

Going to need more information than this. What does npm ls istanbul-reports say?

@jesussobrino
Copy link

I've just tried, and I still have the same issue.

Going to need more information than this. What does npm ls istanbul-reports say?

npm ls istanbul-reports says:

├── istanbul-reports@2.2.6 
└─┬ karma-coverage-istanbul-reporter@2.0.5
  └─┬ istanbul-api@2.1.7
    └── istanbul-reports@2.2.6  deduped

And now it's working like a charm. My previous problem was because I was using istanbul-reports@2.2.4. Sorry, but I didn't understand you properly :)

Anyway, thanks a lot!

@coreyfarrell
Copy link
Member

@MayhemYDG @coderica PR #409 fails (throws) if run against istanbul-report@2.2.5, passes against current master. Actual cobertura output is not tested.

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

No branches or pull requests

7 participants