Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Fix break-on-load to make breakpoints on attach work #332

Merged
merged 5 commits into from
Sep 26, 2018

Conversation

digeff
Copy link
Contributor

@digeff digeff commented Jun 4, 2018

  • Move this._breakOnLoadHelper to common args so break on load can be used for both launch and attach
  • Update the logic of getUrlRegexForBreakOnLoad to handle case insensitive paths
    • Pass caseSensitivePaths around which is an argument of the case insensitive path logic

This as final bug-fixes. At this moment we are using this change as a temporary-workaround to fix the NTVS unit tests

@digeff
Copy link
Contributor Author

digeff commented Jun 4, 2018

@rakatyal PTAL

@roblourens
Copy link
Member

Is this still valid? Are you going to review it @rakatyal?

@rakatyal
Copy link
Contributor

@roblourens: Yes I believe this is still valid but we are not planning to ship this at the moment. I will review this soon.

@digeff
Copy link
Contributor Author

digeff commented Sep 24, 2018

@roblourens @rakatyal We are planning on using this PR for VS 16. Could you please review it now?

@roblourens
Copy link
Member

@rakatyal are you going to review this?

@rakatyal
Copy link
Contributor

@roblourens: Yes I will do this today. Apologies for missing this earlier.

suite('break-on-load', () => {
test('is active when the parameter is specified and we are launching', async () => {
await chromeDebugAdapter.launch({breakOnLoadStrategy: 'regex'});
assert(chromeDebugAdapter.breakOnLoadActive, 'Break on load should we active if we pass the proper parameter and we are attaching');
Copy link
Contributor

Choose a reason for hiding this comment

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

should "be" active


test('is active when the parameter is specified and we are attaching', async () => {
await chromeDebugAdapter.attach({breakOnLoadStrategy: 'regex', port: ATTACH_SUCCESS_PORT});
assert(chromeDebugAdapter.breakOnLoadActive, 'Break on load should we active if we pass the proper parameter and we are attaching');
Copy link
Contributor

Choose a reason for hiding this comment

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

should "be" active

Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and for including so many tests!

@roblourens roblourens merged commit 674f0cb into microsoft:master Sep 26, 2018
@digeff digeff deleted the fix_ntvs_unit_tests branch September 26, 2018 20:47
@roblourens roblourens added this to the September 2018 milestone Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants