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

Add support for screenshots #22

Open
marklagendijk opened this issue Jan 8, 2021 · 7 comments
Open

Add support for screenshots #22

marklagendijk opened this issue Jan 8, 2021 · 7 comments

Comments

@marklagendijk
Copy link

marklagendijk commented Jan 8, 2021

Support for screenshots would be very useful: Azure Devops Pipelines do not support attachments from JUnit reports, but only from TRX reports. See this issue.

The node-trx library has support for attachments. See the code for usage.

@marklagendijk marklagendijk changed the title Support for attachments Add support for screenshots Jan 8, 2021
@pmoleri
Copy link
Contributor

pmoleri commented Jan 15, 2021

No, currently there's no support for attachments/screenshots and I wasn't aware that trx supported this.

I don't see that mocha has support for such feature.
From a very light research I found that:

  • mochawesome adds addContext(this, ...)
  • mochawesome-screenshots adds logReport.setScreenshot(this, ...)

How would your workflow look like? Do you expect mocha-trx-reporter to add a construct like those, so you can attach screenshot for a given test? Or maybe look for files using some file naming convention?

@marklagendijk
Copy link
Author

At first I assume that mocha supported it natively, but this is indeed not the case.
I currently don't know exactly how other mocha reporters solve this, but at some point I found some code in one of them that treated this specifically for different browser testing tools (Cypress, Protractor, WebDriverIO).
However, I can't seem to find it now.

It would be really nice if we could figure out how to do this, because it would help everyone who uses one of these tools in combination with Azure DevOps.
Browser tests can be really annoying if they fail (especially if they fail on the CI pipeline, but not locally), and screenshots can help a great deal with troubleshooting.

@michwii
Copy link

michwii commented Jun 14, 2021

Also interested by the feature !
So difficult to run UI test with Screenshot supports on Azure Pipeline...

@w5l
Copy link

w5l commented Jan 23, 2023

I've ran into the same issue last week, and managed to get this working. So most important, I can confirm that Azure Devops + mocha-trx-reporter / node-trx can attach screenshots to individual tests. Don't listen to anyone saying it doesn't work or you need to use some rest api instead 😉

My setup is using Cypress, logging to cypress-multi-reporters with mocha-trx-reporter and spec (default console) reporter configured. Should also work using mocha-trx-reporter directly though.

I'd like to raise a PR for this but it feels like too much of my fix is Cypress specific, and "magic string" based, so might be out of scope for a Mocha package. My solution works as follows:

  1. Add a Cypress onAfterScreenshot handler (the event only exists on the NodeJS level) and store screenshots paths in a global variable. Trying to do anything else than storing paths fails without visible error. It's also not possible to get a Cypress reference here, or a reference to the test, see After Screenshot API:

    This code is part of the setupNodeEvents function and thus executes in the Node environment. You cannot call Cypress or cy commands in this function, but you do have the direct access to the file system and the rest of the operating system.

  2. Listen to the 'test:after:run' event, which does have a reference to the current test. Grab all logged paths from the global path, store in the test, then clear the global variable. Now the test results received in mocha-trx-reporter contain a list of file attachment paths.
  3. Update the mocha-trx-reporter handler for runner.on('end', ..., and update the testToTrx call to check if the test has attachments. Copy the attachments to the TRX-required magic subfolder, and set the resultFiles property of the TRX test record.
  4. Call Azure DevOps PublishTestResults task on the generated TRX files. I had to explicitly set publishRunAttachments to true even though docs said it defaults to true. YMMV.
       - task: PublishTestResults@2
         condition: always()
         inputs:
           testResultsFormat: "VSTest"
           testResultsFiles: "results/*.trx"
           testRunTitle: "..."
           mergeTestResults: true
           publishRunAttachments: true

If we leave out the Cypress-specific parts of the solution, that leaves a few questions that must be resolved before this can be made into a PR:

  1. I found no default for storing attachments on a test, so I just push file paths to a test.attachments array. Need to know if there is a standard for this, or decide on one. Also, I only push file paths, maybe we want support for a name or title or some other data as well?
  2. I think TRX supports different paths than the ./<trx-file-name>/In/<test-id>/. I found the relativeResultsDirectory in node-trx for that. In my solution however, I tried to stick as close to a "known good" TRX file, so the results directory is always just the default. This is probably more restrictive than needed, but it works. Depending on wishes this should be made more flexible.
  3. I copy all attachments into the root of the test attachment directory (ie. the path above), so it assumes the filenames are unique per test. It also disregards any subdirectories, so attaching /foo/bar/file.png results in ./<trx-file-name>/In/<test-id>/file.png. Need to decide how to deal with duplicate filenames and how to handle folders.
  4. Attachments are copied instead of moved, to not create issues with other code depending on the same files. Might need some switch for move instead of copy, could especially be useful for larger attachments.

@pmoleri @marklagendijk any opinions on this? I'm unsure who the package maintainer is here, sorry.

@pmoleri
Copy link
Contributor

pmoleri commented Jan 24, 2023

Hi @w5l I recently learnt that Azure recommendation is to use JUnit as that's an open format as opposed to .trx which is an internal undocumented format.
Do you think JUnit is good enough to cover all those scenarios?

@w5l
Copy link

w5l commented Jan 24, 2023

The JUnit reporter supports test attachments, and Azure DevOps supports JUnit, but doesn't support attachments defined in JUnit. It's interesting that Microsoft recommends against using their "own" format which has better support.

From microsoft/azure-pipelines-tasks#2058 (comment) (from 2016 but the issue is still unresolved)

My understanding is that the ATTACHMENT syntax above is an extension to JUnit. Uploading other test generated files is on our backlog - for now, you can use the Upload Artifacts task to save any files you may need.

From MSDN https://learn.microsoft.com/en-us/azure/devops/pipelines/tasks/reference/publish-test-results-v2?view=azure-pipelines&tabs=trx%2Ctrxattachments%2Cyaml#attachments-support

The Publish Test Results task provides support for attachments for both test run and test results for the following formats.
(in tabs): Visual Studio Test (TRX), NUnit 3

Since I found no Mocha NUnit reporter, this TRX reporter is the obvious way to go. The underlying node-trx package supports attachments and the generated output is supported by Azure Devops.

@w5l
Copy link

w5l commented Jan 24, 2023

To answer one of my own questions (question 1 above): The mocha-junit-reporter also uses a property attachments on a test which seems to be a an array of string with attachment paths, see:

https://github.com/michaelleeallen/mocha-junit-reporter/blob/4b17772f8da33d580fafa4d124e5c11142a70c1f/index.js#L344-L350

We should be compatible with that, then.

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

4 participants