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

[Record and Playback] Only record the tests that have changed #5156

Closed
sadasant opened this issue Sep 16, 2019 · 5 comments · Fixed by #7213
Closed

[Record and Playback] Only record the tests that have changed #5156

sadasant opened this issue Sep 16, 2019 · 5 comments · Fixed by #7213
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. test-utils-recorder Label for the issues related to the common recorder

Comments

@sadasant
Copy link
Contributor

To reduce the amount of changes per PR, as well as to keep older untraceable tests clean of the folder tree, we should:

  1. Find the set of tests that changed (see note below).
  2. Only record the tests that changed.
  3. Find the recording files that have no matching test.
  4. Remove the files that didn't match any existing test.

Note: To find the changing tests we can either:

a. Use git (either a Node client or child_process) to get the changed files through a diff.
b. Run the playback of each test to see if it doesn't pass, and then add them to the changed-files-pool.
- If so, we need to suppress any console message that might be related to the playback tests before attempting to record them.
- We must also make sure that these playbacks don't add substantial delay to the recordings.

@sadasant sadasant changed the title [recorder] Only record the tests that have changed [Record and Playback] Only record the tests that have changed Sep 16, 2019
@HarshaNalluru
Copy link
Member

HarshaNalluru commented Sep 16, 2019

Some thoughts on how to implement this -
We can leverage the mocha's this context object to do wonders.
We can customize the console logs in the afterEach sections and in case a test fails in during playback, we generate the recording for the test.

Discussion in mochajs/mocha#1998 might be helpful.

@sadasant
Copy link
Contributor Author

This might also be sufficient to close this other issue: #4879

@loarabia loarabia added the EngSys This issue is impacting the engineering system. label Sep 17, 2019
@kurtzeborn kurtzeborn added Client This issue points to a problem in the data-plane of the library. and removed EngSys This issue is impacting the engineering system. labels Sep 19, 2019
@HarshaNalluru HarshaNalluru self-assigned this Jan 22, 2020
@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 22, 2020

My proposal is to run the playback mode for the tests and if a test fails, then record it.

@ramya-rao-a ramya-rao-a added the test-utils-recorder Label for the issues related to the common recorder label Jan 29, 2020
@xirzec xirzec assigned sadasant and unassigned HarshaNalluru Jan 29, 2020
@xirzec xirzec added this to the [2020] February milestone Jan 29, 2020
@sadasant
Copy link
Contributor Author

sadasant commented Feb 3, 2020

What I want to do (though this will change as I dive into the code):

  • Separate the Node Recorder and the Browser Recorder into two files.
  • Separate the code that deals with reading and writing files into re-usable logic, if possible.
  • Separate the code that deals with running each test into re-usable logic, if possible.
  • Write unit tests for the separated methods.
  • See how to best fit a mechanism to prevent recording from happening if no changes have happened.
  • Write the mechanism that prevents recording from happening if no changes have happened.
    • I'm currently thinking on comparing the test that we're running against a previously recorded hash.
    • If no hash exist, record and generate a hash.
  • Write unit tests for the logic that prevents recordings from happening.
  • Make sure everything works in all of the dependencies, but don't commit those recordings and hashes to the PR with the changes.
  • Make a PR with the new recordings and hashes ONLY.

@sadasant
Copy link
Contributor Author

sadasant commented Feb 3, 2020

Ignore the previous points, here's my draft PR: #7213

My current plan is to finish the unit tests and turn it ready for review.

sadasant added a commit that referenced this issue Feb 11, 2020
This PR intends to add soft-record, a way to only record what hasn't changed.

To be able to check whether the tests have changed, we check whether a MD5 hash of the test function's source code has changed from the previous time a recording stored this MD5 to the current test execution.

Any new recording will store this MD5 into the recorded file. To only record the tests that have changed, the user must specify the environment variable TEST_MODE, with value "soft-record".

Fixes #5156
@xirzec xirzec removed this from the Backlog milestone May 18, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. test-utils-recorder Label for the issues related to the common recorder
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants