Fix not hitting BPs on the first line of NTVS unit tests

[digeff]

We were comparing canonicalize vs normal case, so it wasn't matching and we didn't stop on 1,1 because we didn't realize there was a breakpoint there

[roblourens]

What is this unit test testing? Why is it broken now and not before? Details...

[roblourens]

CI?

[digeff]

We have a non-deterministic test. I re-run them and they passed:
2018-10-16T19:33:46.8785757Z 1) ChromeDebugAdapter
2018-10-16T19:33:46.8785830Z Debugger.scriptParsed
2018-10-16T19:33:46.8785898Z tests that sendLoadedSourceEvent will set the reason parameter based on our internal view of the events we sent to the client:
2018-10-16T19:33:46.8785977Z
2018-10-16T19:33:46.8786036Z AssertionError [ERR_ASSERTION]: 'changed' == 'new'
2018-10-16T19:33:46.8786095Z + expected - actual
2018-10-16T19:33:46.8786166Z
2018-10-16T19:33:46.8786250Z -changed
2018-10-16T19:33:46.8786421Z +new
2018-10-16T19:33:46.8786474Z
2018-10-16T19:33:46.8786610Z at sendEventHandler (out\test\chrome\chromeDebugAdapter.test.js:500:32)
2018-10-16T19:33:46.8786684Z at Object.sendEvent (out\test\chrome\chromeDebugAdapter.test.js:93:25)
2018-10-16T19:33:46.8786795Z at ChromeDebugAdapter. (out\src\chrome\chromeDebugAdapter.js:759:27)
2018-10-16T19:33:46.8786922Z at Generator.next ()
2018-10-16T19:33:46.8787017Z at fulfilled (out\src\chrome\chromeDebugAdapter.js:7:58)
2018-10-16T19:33:46.8787094Z at

[roblourens]

I still don't understand what's going on exactly, why is this just an issue now, what changed?

[roblourens]

Is it related to the previous PR? Something else? Just include all the details, people in the future trying to understand why changes happened will thank you.

[digeff]

NTVS unit tests use an "attach" command. We enabled break-on-load there to avoid some race conditions that we had because we cleared all the breakpoints and re-added them.

This bug happens when you put a breakpoint on line 1 of a NTVS unit test. We are going to hit the break-on-load breakpoint and then we are going to see if there is an user breakpoint on line 1, to decide whether to resume or to pause and send this breakpoint to the client. That logic uses getScriptUrlFromId and it was not finding a user breakpoint when there was one, because the case wasn't matching.

This fixes that issue.

[roblourens]

So it only breaks for "attach"? Or no bps on line 1 hit with break on load enabled?

I'll merge it to unblock you

[digeff]

It only breaks with bps on line 1 with with break on load enabled.