Address issue #1495 by adding cypress spec name to mocha suite

[egucciar]

• Closes Reporter - the file (filename) property is undefined #1495

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

• Have tests been added/updated?
• Has the original issue been tagged with a release in ZenHub?
• Has a PR for user-facing changes been opened in cypress-documentation?
• Have API changes been updated in the type definitions?
• Have new configuration options been added to the cypress.schema.json?

[CLAassistant]

All committers have signed the CLA.

[egucciar]

I was able to see this worked for the Test Runner (in open), but i tried to verify this works in Run and It does not appear to be the case :( Will keep digging

[egucciar]

I cant figure this out after 2 hours of going around the code, i guess i wont be splitting my tests in CircleCI by spec name / timings after all

[egucciar]

It kind of works ? I wonder if CircleCI would accept this.

[egucciar]

I wish i could just get it to be one level higher, hm..

[egucciar]

@brian-mann i dont want to be much of a bother, but could anyone take a look at this PR / leave feedback / assist or provide guidance in updating these snapshot tests?

[jennifer-shehane]

@egucciar It may be best to start by writing tests for the behavior your want - at least in this way, we can know what your end result should be and can offer more guidance.

I'm also seeing the standard out printing this 'has file:' text in every single standard out now (captured in the failing snapshot tests), regardless of the use of a reporter. This is definitely not something we want printed in this way and I'm not sure where this text is coming from.

[egucciar]

@jennifer-shehane thanks for the quick response. The has file: log comes from this commit from Chris Breiding 31 months ago:

1d2383a

I shall comment it out.

I'm trying to run reporter_spec.coffee as this is where I'd add an additional assertion for the file i added (I am checking with CircleCI if this is sufficient - as the code stands today, adding the file somewhere in the report works but I'm unsure if it works for CircleCI if its not in a specific spot). As can be seen in the screenshot of the generated report, this 1 parent nesting, and 2 children testsuites, and the place where file is outputted is on a line with no test timing information so I'm feeling it wont work.

But, I can't get the test to run locally when doing this:

This is happening for me also on the develop branch, after having built all the packages, and reinstalling the . node modules

Unfortunately without a baseline of passing unit tests I feel uncomfortable updating the existing test with the new assertion just yet.

[egucciar]

@jennifer-shehane thanks so much for the tip with the log, because that fixed all the tests!!! YAY!

But, I agree with your update that this is still a WIP because i believe the file should be on a Line that has the actual test information, i think i know what i can still try to do to fix this:

But I'll have to look into it after our next quick launch, a few days.

Basically, digging deeper into this LOC i added here to ensure it is added in the "right place" since i believe the reporter is just a pass-through and this only adds it on the one place where i added it:

[thewalla07]

@egucciar any update on this PR?

[egucciar]

I don't have an update. I would really like this feature to be incorporated but i was more or less taking stabs in the dark and couldn't get it over the finish line. I don't recall what specifically blocked me.

[gabbersepp]

@egucciar I am willing to finish this PR as I want to have the filename to be included in the test results. We use jenkins and a custom reporter and without the filename we have to to a full-text-search through all files to find the spec file that contains a failed test.

To do this it would be nice if you can tell me, what is currently missing and what is the current state? Reading through the comments does not help me unfortunatelly :-(

[egucciar]

@gabbersepp Take a look at a mocha suite with timestamps generally used by CircleCI...the time stamp has to be present on the same tag that has the name of the file I think for CircleCI to correlate the time to the file name.

I think my change may have propogated the file name to the output, but as can be seen in the screenshots not on the same tag as the timing.

[egucciar]

@gabbersepp if you just need a filename i recommend taking my work, rebasing and seeing if it fits your needs. I needed the filename and timestamp to be in the same tag and couldn't figure out how to write test cases or get the code to do what I want because doing this very simple 2 line change took me hours.

[egucciar]

also, i do remember what blocked me. couldn't get the tests to run, did not receive any help or support on being able to run them. The comment where i showed the issue with the running test is where i left off. I was able to get the log as pointed out by jennifer to disappear from the circle output, but the tests still would not run locally. Never did get a response to that comment and couldn't proceed.

[gabbersepp]

thanks for your feedback @egucciar . Should be enough information for me :-)

[gabbersepp]

@egucciar I cherry-picked your commits into a new branch: https://github.com/gabbersepp/cypress/tree/issue-1495
I also was able to access the file attribute within the reporter.

To fully understand your problem:

You want the filename not only to be on the first testsuite tag but also on the second one?

[egucciar]

Right, exactly, where the timing and test info are

[gabbersepp]

I made some research but did not found any documentation that declares file to be part of the official junit xml format.
E.g.: https://github.com/junit-team/junit5/blob/master/platform-tests/src/test/resources/jenkins-junit.xsd
Unfortunatelly I am neither familiar with circle ci nor with junit xml format.

Maybe the solution is to to use the filename as testsuite name? I took a quick look into the jest-junit project and they introduced an own flag for controling whether or not the filename should be mapped to the suite name.

[egucciar]

It seems as though it was once supported in an old version of mocha junit.

michaelleeallen/mocha-junit-reporter@673aef9

Honestly, this whole thing is confusing. Basically, the filename is the only was in which we can pass to Cypress what specs to run. Thus, split by filename would be the only split type we could leverage. Seems as though the expected behavior from circle would be similar to classname. Not sure where the mocha junit spec is documented or enforced.

circleci/circleci-docs#1837

[gabbersepp]

making this change in the junit-reporter:

Results in:

So this change had to be made in the reporter project (https://github.com/michaelleeallen/mocha-junit-reporter) or you @egucciar write an own reporter that can do this.

[gabbersepp]

It seems as though it was once supported in an old version of mocha junit.

michaelleeallen/mocha-junit-reporter@673aef9

Honestly, this whole thing is confusing. Basically, the filename is the only was in which we can pass to Cypress what specs to run. Thus, split by filename would be the only split type we could leverage. Seems as though the expected behavior from circle would be similar to classname. Not sure where the mocha junit spec is documented or enforced.

circleci/circleci-docs#1837

Please have a look at my comment. It was to the same time as yours so maybe you missed it. :-)
I think this old commit addresses another usecase as it puts the file attribute to the root suite I think.

[egucciar]

Okay. I think I will also close this PR. I don't really have the capacity or need to work on this right now, but timings data would be interesting to me in the future. If it requires work elsewhere then it certainly doesn't need a PR here. My only question is if that change works by itself without any changes to Cypress ? Or alongside the change I made here ?

[gabbersepp]

Alright well I'll need to merge develop in. Dunno why the builds aren't running either. Also I can remove the WIP tag. It accomplishes as much as it can I suppose!

Thanks! I assume that the remaining build chain stopped because of the WIP tag. But let's wait until the builds have finished.

[gabbersepp]

All builds are gree. Looks good!
Let me know if you need further assist.

[jennifer-shehane]

@egucciar Can you resolve conflicts and address this comment please? #3960 (review)

If you are unable to work on this PR anymore, please close the PR.

[egucciar]

Hi @jennifer-shehane , I don't know if you saw but I responded to your comment. Please let me know if removing the commented lines of code makes sense given my answer to your question.

[egucciar]

@jennifer-shehane resolved conflicts and removed the commented out lines of code assuming that's the right thing to do here.

Addressed changes for comments

[cypress]

---

Test summary

3607 • 0 • 47 • 0

---

Run details

Project	cypress
Status	Passed
Commit	3af86d1
Started	Jan 9, 2020 6:37 AM
Ended	Jan 9, 2020 6:42 AM
Duration	04:38 💡
OS	Linux Debian - 9.11
Browser	Multiple

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[egucciar]

Thanks @chrisbreiding for pushing this over the finish line 🙌

[rtymchyk]

There was some conversation around making this compatible with CircleCI. It seems like the test results are now getting properly populated with filenames, and I can see it in the test reports. However in CircleCI I am still getting an error:

Error autodetecting timing type, falling back to weighting by name. Autodetect no matching filename or classname.  If file names are used, double check paths for absolute vs relative.
Example input file: "cypress/tests/contact.tests.js"
Example file from timings: ""

Just wondering if anyone had any luck with setting up parallelization in CircleCI with this test metadata?

[jrnail23]

@rtymchyk, I've tried it on my own, using their cli for test splitting, and the problem seems to be, at least in my case, that the timings data file (/.circleci-task-data/circle-test-results/results.json, by default) is missing file paths for each of my tests (file: null). To be clear, the rest of the tests' attributes are populated, but file is null.

[collinbachi]

We are also still having trouble getting CircleCI to map the filenames properly

[egucciar]

For any still interested, i have an interest in trying this again by forking the reporter as @gabbersepp suggested and kindly demonstrated what file changes i could make to accomplish the goal.

[egucciar]

Following @gabbersepp 's advice, one can modify the default mocha-junit-reporter to match CircleCI's expectations for XML. I have done this in the local project and may extrapolate this further to a forked repo and PR for mocha-junit-reporter, but will need to determine if that is a course of action I'll be taking. To assist others who are willing to do a similar fork process, this is what I ended up changing:

I am able to see partial CircleCI data come through. I am absolutely getting SOME timing data with SOME files now, at least, and pretty sure the rest may have to do with circle. Circle requires green build for timing data so still working through why missing data for some of the specs...not sure if its related to the fact that with parallel jobs, it seems to only pick the result from "one" container. Hence I will work with CircleCI if this doesnt do the trick 100%, but im confident this does solve the issues on the XML reporting side.

• testsuite needs timing and filename data
• I think - i could be wrong - the Root suite messes things.