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

Fix terminal link tests #34193

Closed
Tyriar opened this issue Sep 12, 2017 · 7 comments · Fixed by #40370
Closed

Fix terminal link tests #34193

Tyriar opened this issue Sep 12, 2017 · 7 comments · Fixed by #40370
Assignees
Labels
debt Code quality issues terminal Integrated terminal issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Sep 12, 2017

Disabled in 128a4e3

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues labels Sep 12, 2017
@Tyriar Tyriar added this to the September 2017 milestone Sep 12, 2017
@Tyriar Tyriar self-assigned this Sep 12, 2017
Tyriar added a commit that referenced this issue Sep 12, 2017
@Tyriar
Copy link
Member Author

Tyriar commented Sep 12, 2017

@timbanaveen In order to fix a VS Code crash caused by a regex catastrophic backtracking case, I changed the unix link link regex (#34039). I had to disable some of the terminal link line/column Unix cases (added in #24832) as I don't totally understand what's happening. For some reason unixLineAndColumnMatchIndex doesn't seem to work for all the lineAndColumnClause anymore, specifically these ones:

	'((\\S*) on line ((\\d+)(, column (\\d+))?))', // (file path) on line 8, column 13
	'((\\S*):line ((\\d+)(, column (\\d+))?))', // (file path):line 8, column 13

See this change which gets 2/4 cases working: b411fb2

Any insights would be great 😃

@timbanaveen
Copy link

@Tyriar, just saw this issue and fix. will have a detailed look in this on weekend, will update you on any findings.

@Tyriar Tyriar modified the milestones: September 2017, October 2017 Sep 26, 2017
@Tyriar
Copy link
Member Author

Tyriar commented Sep 26, 2017

@timbanaveen did you get a chance to look at this?

@timbanaveen
Copy link

@Tyriar got new machine, i have just started setup. Will update soon.

@timbanaveen
Copy link

@Tyriar I tried running test from terminal using scripts\test.bat, but it seems to be stuck, shows nothing on terminal. Is there any way to run specific test or something i am missing.
Thanks

@Tyriar
Copy link
Member Author

Tyriar commented Oct 10, 2017

Hmm... I'm sure there is a way as it uses mocha, I'm not sure what it is though as the tests always run so fast for me.

@timbanaveen
Copy link

@Tyriar I have updated the tests and ts file, now tests are working fine. Please review. Thanks

@Tyriar Tyriar modified the milestones: Backlog, December 2017/January 2018 Dec 18, 2017
Tyriar pushed a commit that referenced this issue Dec 18, 2017
@Tyriar Tyriar added debt Code quality issues and removed bug Issue identified by VS Code Team member as probable bug labels Dec 18, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues terminal Integrated terminal issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants