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

Replace mockery to sinon library in mock-run which is used for Tasks Unit Tests #961

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

LiliaSabitova
Copy link

@LiliaSabitova LiliaSabitova commented Aug 15, 2023

This PR includes changes that are made to the azure-pipelines-task-lib to replace the Mockery package with Sinon. Despite not utilizing the vulnerable method in Mockery (CVE-2022-37614 in Mockery 2.1.0), it was decided to make this change because Mockery is outdated. Sinon appeared to be the most suitable package. The changes proposed in this PR affect the unit tests of Pipeline Tasks, and some tasks may need adjustments when updating to the new version of task-lib.

Changelog:

  1. Replaced Mockery with Sinon: Sinon has been added as a new dependency, replacing Mockery in the project.
  2. Refactoring TaskMockRunner: TaskMockRunner has been refactored to accommodate the changes brought by the transition from Mockery to Sinon. The new methods have been adjusted accordingly.

Impact of Changes:

The changes require additional alterations in several situations when updating Pipeline Tasks to the new version of task-lib:

  1. Relative Path: If a relative path was used for mocking a local module, changes are required.
  2. Import Aliasing: If import aliasing was used in the task implementation, and it is being mocked in unit tests, changes are needed.
  3. Initialization Order: Changes may occur because the sequence in which you initialize the task-lib and set inputs in tests is important. Setting inputs should go before initialization of task-lib.
  4. Module Clone and Recursive Calls: To maintain the original behavior within a mocked function in Sinon, you need to add module's clone, otherwise it may lead to recursive calls.
  5. Localization Changes: If a localized error message was used, it will now go through a mocked loc function and hence, get a mocked localization, not the real one.

Testing:

The changes were thoroughly tested using the unit tests of tasks under our ownership in the Pipeline Tasks repository. Out of around 200+ tasks, half are under our ownership. Out of these, not all are affected by these changes. Around 40-50 tasks that were affected have been tested.

Documentation:

After the PR gets merged, the documentation for the TaskMockRunner class should be updated to reflect the new and refactored methods and the usage of Sinon.js over Mockery. It should provide examples of how to use the new methods to accommodate the changes made.

Note:

This PR doesn't directly handle the localization changes, module initialization, relative path adjustments, recursive calls, and import aliasing. A separate migration guide will be released to help others navigate the necessary modifications in their tasks.

node/mock-run.ts Show resolved Hide resolved
node/mock-run.ts Outdated Show resolved Hide resolved
node/mock-run.ts Outdated Show resolved Hide resolved
node/mock-run.ts Show resolved Hide resolved
LiliaSabitova and others added 3 commits August 16, 2023 14:04
change to precise types in registerMock

Co-authored-by: Konstantin Tyukalov <[email protected]>
change to console.log because it causes tests to fail
@lgmorand
Copy link

👀 eagerly waiting for this to be merged <3 :)

@jessehouwing
Copy link
Contributor

Fixes: #955

cat2608 added a commit to snyk/snyk-azure-pipelines-task that referenced this pull request Sep 14, 2023
@DergachevE DergachevE force-pushed the users/LiliaSabitova/MockeryReplacementToSinon branch from 860f6e6 to e0a8988 Compare April 26, 2024 10:23
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

Successfully merging this pull request may close these issues.

4 participants