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

git: skip failing git unit test on win #7068

Merged
merged 1 commit into from
Feb 5, 2020
Merged

git: skip failing git unit test on win #7068

merged 1 commit into from
Feb 5, 2020

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes #7064

Fixes the failing git unit test by making it more robust to problems caused as described in #933.

Note:

I understand that the original error might require more fundamental changes to git for Windows but since we will soon be migrating to the builtins, I thought it'd be low effort to get the CI to pass successfully.

How to test

Verify that the tests during CI for @theia/git successfully pass.

Review checklist

Reminder for reviewers

Signed-off-by: Vincent Fugnitto [email protected]

@vince-fugnitto vince-fugnitto added git issues related to git ci issues related to CI / tests labels Feb 4, 2020
@vince-fugnitto vince-fugnitto self-assigned this Feb 4, 2020
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

CI is green again, LGTM.

@paul-marechal
Copy link
Member

Although, is it really normal to get duplicate repositories back?

@vince-fugnitto
Copy link
Member Author

Although, is it really normal to get duplicate repositories back?

No but it looks to be a complex issue #933 with Windows.
I thought that since @theia/git would be deprecated soon we should fix the CI for the moment.

@vince-fugnitto
Copy link
Member Author

@kittaakos are you against such a change?
I understand we lose the exactness of the unit test but I thought that it could be a temporary measure since:

  • it fixes the failing CI for all new pull-requests
  • the @theia/git will be removed in favor of vscode-builtin-git and vscode-builtin-git-ui in the future
  • the problem [git][Windows] Repository locator test failure on Windows #933 is quite old and has yet to be resolved which leads me to conclude it is a difficult and complex problem

@kittaakos
Copy link
Contributor

are you against such a change?

No, I do not, but I'd need a little bit more time to dig into; next week 😕 As a quick fix for now, can we skip the test on Windows?

@vince-fugnitto
Copy link
Member Author

are you against such a change?

No, I do not, but I'd need a little bit more time to dig into; next week 😕 As a quick fix for now, can we skip the test on Windows?

Sure, I can update the pull-request to skip the test on Windows for the moment :)

@kittaakos
Copy link
Contributor

Sure, I can update the pull-request to skip the test on Windows for the moment :)

It's your call; whichever is easier for you. Skipping the test on Windows seems to be a bit more straightforward "fix".

@vince-fugnitto vince-fugnitto force-pushed the vf/git branch 2 times, most recently from 077394e to 4f87cd9 Compare February 5, 2020 14:24
Fixes #7064

Skips the failing `@theia/git` unit test on Windows (#933).

Signed-off-by: Vincent Fugnitto <[email protected]>
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you for taking care of the CI, @vince-fugnitto 👍

Please proceed with the merging once you see the green builds.

@vince-fugnitto vince-fugnitto merged commit 8deef2a into master Feb 5, 2020
@vince-fugnitto vince-fugnitto deleted the vf/git branch February 5, 2020 15:24
@vince-fugnitto vince-fugnitto changed the title git: make git unit test more robust git: skip failing git unit test on win Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests git issues related to git
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ci: build failure on windows during @theia/git unit test
3 participants