Add support for test artifacts (videos and screenshots)

[rotemmiz]

We want to add the following to an artifact directory for each failed test:

1. Video of the test from start to finish (using fbsimctl record) - New details below
2. Device log (can be extracted from ~/Library/Logs/CoreSimulator/{udid}/system.log - Implemented
3. Print hierarchy with testIDs (we already print that to log on a failed expectation)
4. Screenshots (I actually believe a video will have a much higher value in understanding what went wrong)

EDIT:
Detailed requirements and implementation guide:

iOS

1. capturing video can be done using xcrun simctl io udid recordVideo <filename>.<mp4|mov>, this functionality needs to be added to the wrapper class AppleSimUtils. This should be implemented using sapwn , and not exec since this process is not detaching, it actually listens to sigterm to finish its job, so it can be implemented in a similar way as spawnEmulator
   
   Similarly, add screenshot functionality using xcrun simctl io booted screenshot, add it to the same place.
   
   Add takeScreenshot and recordVideo empty functions in DeviceDriverBase
   Implement both functions in SimulatorDriver's takeScreenshot and recordVideo functions.

Android

1. Taking a screenshot with Android - add to ADB
2. Taking a sceenrecord - also needs to initiated with spawn - add to ADB
3. Implement both functions in AndroidDriver's takeScreenshot and recordVideo functions - this will also add support for screenshot and video taking on Android Devices.
4. we would also need to add log fetching from adb logcat , in the same scope as we do on iOS, for feature parity.

API

The newly generated files need to be arranged in folders arranged by test names. There is already an internal tool for that called ArtifactsCopier. The current implementation is a bit lacking since every artifact type needs to be added manually.
In the scope of this task I would also like to fix this, adding an internal API to register new artifact types (suggestions are welcome)...
When work is done and artifacts are registered into ArtifactsCopier, each test should generate 3 artifacts:

1. device log in the scope of the test
2. screenshot at the end of the test (maybe another one at the begining of the test as well ?)
3. video screen capture of the entire test.

[LeoNatan]

Not just for failure. We should support artifacts in general.

[isnifer]

@rotemmiz sometimes screenshots enough. In most cases, video don't need

[LeoNatan]

Artifacts could include the app’s log, screenshots, video of the test run, profiling data, etc. After the test suit finishes, we need to find a way to take this data from the device and transfer it for export. On simulator, this is easy, but it may be challenging on real devices. Xcode is able to take an app’s container data and export it in an archive. Perhaps we could use this somehow.

[fabriziogiordano]

What about just for now use xcrun simctl io booted screenshot testId.png in the mochajs tests?

[rengarima]

Hi Is there an update on this/ETA or a workaround - My tests fails for proxy errors on the server and I would like to take screenshots while its running on the CI to rule out proxy issues.

[dzunglht-ibl]

Please support this ticket guys, we expect detox will be the first pioneer all-in-one toolkit for QA/automation things dedicated in RN world.
Tons of thanks

[OisinOKeeffe]

@fabriziogiordano I'm interested in your suggestion regarding running xcrun simctl io booted screenshot testId.png in the mochajs test for the time being. Did you get that to work? can you show us how?

[OisinOKeeffe]

FYI if anyone was lost like I was I was able to get this working on iOS creating the following helper:

const { exec } = require('child-process-async');

module.exports = async (elementToBeVisible, fileName) => {
  await waitFor(element(by.id(elementToBeVisible)))
    .toBeVisible()
    .withTimeout(5000);

  await exec(
    `xcrun simctl io booted screenshot e2e/screenshots/${fileName}.png`
  );
};

[rotemmiz]

Thoughts on our current state, and how I see this feature:

I wasn’t able to merge the current implementation since I felt like it’s not scalable enough. Meaning, my initial approach, the one I asked @donataswix to implement (mentioned at the top of this issue), has a serious flaw. It is very hard to add a new artifacts to the system.

Android logs, for instance, were not implemented in this branch, and it seems like it’s going to be very hard to add this capability, let alone adding more artifacts in the future, will cause a code overflow in deviceDrivers.
ArtifactsCopier+ArtifactsPathProvider themselves needs a bit refactoring, and should be merged into one class (?) and maybe name change as well (?), so that it will be able to fully control the artifact creation lifecycle, including file naming and triggering start, stop, move, copy on each artifact handler.

Each ArtifactHandler will have a single responsibility, implementingstart, stop, move, copy methods, for each artifact type.
Let’s take android video artifact handler for example, it should extend the functionality of AndroidArtifact to not only handle copying and moving, but control the whole thing :


class AndroidVideoArtifactHandler {
  ...

  async start() {
    //spawn video process creation here
  }

  async stop() {
    //stop the spawned process
  }

  async move() {
    //as currently implemented in AndroidArtifact
  }
  

  async copy() {
    //as currently implemented in AndroidArtifact
  }
}

something like that, not sure its a good enough approach, we need to test it. I’d like your inputs on that @donatas.
I think I need your help with explaining the caveats, since you’re the only one who encountered all of them, and maybe help designing it.

[LeoNatan]

More considerations in the future:

• When we find time to integrate Detox Instruments with Detox, we will need a mechanism to transfer files from and to devices.
• When we decide to add support for real devices, we will need a mechanism to transfer user notifications and user activity files to the devices.
  • Android emu or device will likewise need that functionality for notifications

Before implementing artifacts, I think we need to first implement the file transfer infra for each platform.

[donataswix]

Agree with @rotemmiz, current implementation in https://github.com/wix/detox/tree/screenshots-and-videos should be improved. Drivers got polluted with data that could be easily contained in artifact handlers, e.g. AndroidVideoArtifact could hold current process object and handle it's creation and termination. Also, currently Device object is orchestrating recording of screenshots and videos, but these could be delegated to ArtifactsManager which should subsume and replace ArtifactsCopier. New options (--record-videos and --take-screenshots) should be passed as parameters to this ArtifactsManager, thus freeing Device of this knowledge and logic.

What Device should do in the end is to tell ArtifactsManager what to record and what not to (--record-videos and --take-screenshots), pass list of DeviceDriver's artifacts to ArtifactManager, and then tell it when to start, stop, and copy or move those artifacts.

[noomorph]

Hello to all interested,

I indeed started my work on the feature, and the current challenge is to refactor it properly.

As it was stated previously, we want to avoid tight coupling of Detox internals with the test artifacts feature, since most of it is about spawning and controlling a handful of CLI processes at the right times.

Also, we might benefit from making methods takeScreenshot('screenshot name') and recordVideo('video name') public, because it opens a possibility for visual comparison tests and other creative uses.

Last but not least, in the interim I've noticed that in the artifacts branch Jest support was overlooked.
As you may know, the artifacts feature, as of right now, relies on the knowledge of current test name and status. Incidentally, it took me a couple of days to figure out a satisfactory way to convey current test name and status into beforeEach and afterEach functions, and it significantly differs from Mocha way, to my great displeasure.

This omission concerns me in a sense that Detox, being agnostic to test frameworks and runners, at the moment has no explicit contract for an arbitrary test runner. It is true that we did not need that in the past.
In fact, as a user, you were required only to call detox.init(config) in the very beginning and detox.cleanup() in the end.

Unfortunately, the new artifacts feature needs more points of interaction, for instance:

• before test runs, get current test name and make the runner wait till the video recording process is successfully spawned;
• after the test ends, stop video recording (which is also an async op), ascertain the status of the test (whether it passed or failed), and according to the preferences, decide whether we keep the video or delete it;
• since "keeping video" in a case with Android assumes copying files from device (or emulator) filesystem to the local artifacts folder, we'd want to perform that in the background (to avoid blocking the next test execution), but ultimately we'll still need to wait for the all copy operations to complete before we allow the runner itself to terminate.

Altogether, it brought me to the following stance:

1. We should split the artifacts feature into two parts:

   1. provide imperative API for taking screenshots and recording videos
   2. integrate this API with CLI options and test runner lifecycle in a scope of a separate feature.
      I am confident that both parts can be developed independently from each other.

2. While 1.i is a relatively standalone feature, we need to make several preparations before doing 1.ii:

   1. Define Detox <-> Test Runner contract:

      1. detox init -r runnername implementation for scaffolding initial Detox suite with a proper runner config.

      2. detox test -r runnername partial implementation. It regards the part which prepares command-line arguments, environment variables and whatever the runner needs to work, and triggers the process synchronously. Something like (detoxConfig) => cp.execSync("runnername --runnerArgsBuiltFromDetoxConfig...").

      3. within test runner execution context, piece detox options together into a single detoxConfig.
         While it is not self-evident, but, for instance, with Jest (which rejects unknown flags), we are forced to pass detox options via environment variables, while with Mocha, we are freely handing command-line arguments as they are. That means that Jest integration should mostly look into process.env, while Mocha integration should be parsing process.argv. To my taste, I would prefer two different and clear implementations, so to say, instead of generic know-it-all detox/src/utils/argparse.js.

      4. provide either a method like setup(detoxTestRunnerHooks), or a particular module which ensures that:

         1. Like in a turn-based game, Detox gets its time to make its moves:
            1. on start, before any test runs;
            2. before a test;
            3. after a test;
            4. on exit, when all tests have already run.
         2. Detox gets information about the current test:
            1. before a test: test name, test full name (optional).
            2. after a test: test name, test status (passed or failed), test full name (optional).

         I'm still hesitant about a way to achieve the functionality above. For example, if we go with a setup() method, its encapsulated nature is going to contradict the current Detox approach of straightforward and open to editing setup files like e2e/init.js that Detox scaffolds upon init.
         On the other hand, if we go with extending init files, there are problems as well. In fact, I could not meet the requirements above (a.a-a.d,b.a-b.b) with less than 50 lines of code for Jest, and that, by itself, ruins the simplicity of startup files and complicates introducing changes in future to these files as they might grow and differ from project to project. This is a place where I'm stuck currently and thinking on a solution. UPD (04/26/18): I've managed to pack the workflow into 18 lines here: https://github.com/noomorph/jest-hooks-poc/blob/master/e2e/init.js . So, this option can work out too.

   2. Extract detox integrations with mocha and jest into two separate modules, e.g.: detox-runner-mocha and detox-runner-jest.

   3. Provide minimum viable API for Detox extensions (plugins):

      1. CLI and package.json options registration and their values' retrieval in runtime.
      2. asynchronous onStart, beforeTest, afterTest, onExit hooks with test information that can be used.
      3. provide runtime info (like current platform, various executables paths) and convenience helper methods to the plugins.

3. Completing the artifacts feature:

   1. Optionally, extract the whole artifacts feature as a plugin using API in 2.iii.
   2. Alternatively, hard-code CLI hooks without doing 2.iii and wire the methods like (.recordVideo() and .takeScreenshot()) straight to the place where detox gets control from test runner in (before|after)(All|Each) calls. I don't favor this way, but it can serve us as a transit point to the cleaner integration in the future.

It's a bit hard to track my progress since it is quite chaotic movement here-and-there, which is uniformly and messily distributed between three branches: feature/detox-artifacts-plugin, refactor/test-runners, and the original one, plus some local files and quite a recent Jest PoC.

I'll do my best to make my scope of work more focused in the nearest days and will be gladly updating you and listening to your input.

Cheers!

[noomorph]

Quick update, guys. We've found a sweet spot between what is best and what is just good enough.

The work is going on in this (last and final) artifacts branch: https://github.com/wix/detox/compare/feature/test-artifacts . To great relief, there's no need to scatter work across branches - I have a focus now (at last :).

Within next couple of days, I'll create a new WIP Pull Request instead of #541 , so that my progress will be more visible and transparent to us all. I'll make sure to create a checklist in the PR description and keep it in sync with my progress.

I have a cautious hope to finish work on artifacts quite soon. Still, besides the parts I clearly understand how to accomplish, there are less predictable ones: infrastructure work to improve iOS log recording code, merging low-level parts of iOS screenshotting and screen recording from the former pull request. Maybe we'll postpone implementing Android log recording. Anyway, a few big challenges on structuring have been solved, as well as integration with both Mocha and Jest (as a bonus, detox init -r jest was also implemented).

Many thanks for your patience!
Stay tuned.

[noomorph]

@rotemmiz , can we close this?