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 regression test for .git file watcher #3259

Merged
merged 5 commits into from
Feb 11, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Feb 9, 2023

Followup to #3191

@julieg18 julieg18 self-assigned this Feb 9, 2023
@julieg18 julieg18 marked this pull request as ready for review February 10, 2023 00:04
@@ -74,7 +79,7 @@ export class ExperimentsData extends BaseData<ExperimentsOutput> {
this.dvcRoot
)

const dotGitPath = getGitPath(gitRoot, gitPath.DOT_GIT)
const dotGitPath = this.dotGitPath || getGitPath(gitRoot, gitPath.DOT_GIT)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Q] Are you sure you need this when AvailableCommands.GIT_GET_REPOSITORY_ROOT is already returning the mockRoot? Could you stub getGitPath instead of adding code just for testing purposes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

extension/src/test/suite/experiments/data/index.test.ts Outdated Show resolved Hide resolved
extension/src/test/suite/experiments/data/index.test.ts Outdated Show resolved Hide resolved

const data = disposable.track(
new ExperimentsData(
dvcDemoPath,
{
dispose: stub(),
executeCommand: mockExecuteCommand
executeCommand: (command: CommandId) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up moving the executeCommand to be inline to avoid the no-identical-function eslint rule. I could be wrong, but when I tried to create a single function for the two to share it looked like I was overcomplicating things 🤔

@codeclimate
Copy link

codeclimate bot commented Feb 10, 2023

Code Climate has analyzed commit ad931a1 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.9% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit 3bc50b2 into main Feb 11, 2023
@julieg18 julieg18 deleted the add-regression-test-for-git-file-watcher branch February 11, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants