Skip to content

Commit

Permalink
Fix breakpoint sync issues (#143)
Browse files Browse the repository at this point in the history 
* support marking breakpoints as `pending`

* Implement `deleteBreakpoint` for bp manager

* retain bp "pending" status if bp is deleted

* Fix breakpoint deviceId issues

* Only run single breakpoint sync at a time

* BreakpointRef support

* Fix failing test

* Prevent tests from stalling out

* Resurrect breakpoints when device failed to remove

* Fix bp resurrection

* Make the bp events more generic

* Add unit test for removing failed add breakpoints

* Update changelog for v0.18.9

* 0.18.9

* Update changelog for v0.18.10

* 0.18.10

* Fix crash by using postman-request (#151)

* Update changelog for v0.18.11

* 0.18.11

* Fix `file already exists` error and hung process (#152)

* Remove axios in favor of postman-request (#153)

* Update changelog for v0.18.12

* 0.18.12

* File logging (#155)

* Adds FileLoggingManager

* Fix missing cwd

* Update changelog for v0.19.0

* 0.19.0

* Merge branch 'master' of https://github.com/rokucommunity/roku-debug into DebugProtocolServer

* Simplified the relay session test

* Move @types/request to deps to fix d.bs files

* Update changelog for v0.19.1

* 0.19.1

* Support sgrendezvous through ecp (#150)

* A lot of foundational work

* Update testing

* Push some of the more foundational functions

* Add minversion function and await to an async function

* Ending curly bracket

* Add types

* Add add launch config info to device info

* Capture rendezvous

* Wrap up rendezvous support

* delete log

* Make code more easily testable

* Change a few things around how to handle the rendezvous data

* test pingEcpRendezvous

* Fix bug with telnet and ecp mismatch

* Drive usage based on launch config setting instead of device.
Move rendezvous events out of adapters.

---------

Co-authored-by: Bronley Plumb <bronley@gmail.com>

* Update changelog for v0.20.0

* 0.20.0

* Fix rendezvous crash (#156)

* Fix timing bugs during rendezvous tracking startup

* Only emit rendezvous data if new data was received

* Update changelog for v0.20.1

* 0.20.1

* Bump word-wrap from 1.2.3 to 1.2.4 (#157)

Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4.
- [Release notes](https://github.com/jonschlinkert/word-wrap/releases)
- [Commits](jonschlinkert/word-wrap@1.2.3...1.2.4)

---
updated-dependencies:
- dependency-name: word-wrap
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update changelog for v0.20.2

* 0.20.2

* Add option to delete dev channel before install (#158)

* Update changelog for v0.20.3

* 0.20.3

* When a breakpoint fails to delete because of error NOT_STOPPED. Store
the breakpoints and delete later.

* Store the srcHash and destHash as different hashes
Create a mapping of destHash to breakpoint deviceId

* Unit test fixes

* Fix another test

* Remove more bp resurrection tests

* Always set deviceId for bps, even on error

* Remove unnecessary cache and resurrection references

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Milap Naik <milapnaik@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Christian Holbrook <cholbrook@fubo.tv>
  • Loading branch information
@TwitchBronBron @MilapNaik
@dependabot @Christian-Holbrook
4 people committed Sep 20, 2023
1 parent 38ff9b8 commit f1b16f7
Show file tree
Hide file tree
Showing 15 changed files with 652 additions and 530 deletions.
23 changes: 17 additions & 6 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"watchFiles": [
"src/**/*"
],
"timeout": "2000",
"timeout": 2000,
"fullTrace": true,
"watchExtensions": [
"ts"
Expand Down Expand Up @@ -60,15 +60,16 @@
},
"devDependencies": {
"@types/chai": "^4.2.22",
"@types/debounce": "^1.2.1",
"@types/dateformat": "~3",
"@types/debounce": "^1.2.1",
"@types/decompress": "^4.2.4",
"@types/dedent": "^0.7.0",
"@types/find-in-files": "^0.5.1",
"@types/fs-extra": "^9.0.13",
"@types/glob": "^7.2.0",
"@types/mocha": "^9.0.0",
"@types/node": "^16.11.6",
"@types/request": "^2.48.8",
"@types/semver": "^7.3.9",
"@types/sinon": "^10.0.6",
"@types/vscode": "^1.61.0",
Expand Down Expand Up @@ -96,7 +97,7 @@
},
"dependencies": {
"@rokucommunity/logger": "^0.3.3",
"brighterscript": "^0.65.4",
"brighterscript": "^0.65.0",
"dateformat": "^4.6.3",
"debounce": "^1.2.1",
"@types/request": "^2.48.8",
Expand Down
159 changes: 151 additions & 8 deletions src/adapters/DebugProtocolAdapter.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { RemoveBreakpointsResponse } from '../debugProtocol/events/responses/Rem
import { BreakpointVerifiedUpdate } from '../debugProtocol/events/updates/BreakpointVerifiedUpdate';
import { RemoveBreakpointsRequest } from '../debugProtocol/events/requests/RemoveBreakpointsRequest';
import type { AfterSendRequestEvent } from '../debugProtocol/client/DebugProtocolClientPlugin';
import { GenericV3Response } from '../debugProtocol/events/responses/GenericV3Response';
import { RendezvousTracker } from '../RendezvousTracker';
const sinon = createSandbox();

Expand All @@ -45,6 +46,13 @@ describe('DebugProtocolAdapter', function() {
let client: DebugProtocolClient;
let plugin: DebugProtocolServerTestPlugin;
let breakpointManager: BreakpointManager;
let projectManager: ProjectManager;
let deviceInfo = {
'software-version': '11.5.0',
'host': '192.168.1.5',
'remotePort': 8060
};
let rendezvousTracker = new RendezvousTracker(deviceInfo);

beforeEach(async () => {
sinon.stub(console, 'log').callsFake((...args) => { });
Expand All @@ -56,7 +64,7 @@ describe('DebugProtocolAdapter', function() {
const locationManager = new LocationManager(sourcemapManager);
const rendezvousTracker = new RendezvousTracker({});
breakpointManager = new BreakpointManager(sourcemapManager, locationManager);
const projectManager = new ProjectManager(breakpointManager, locationManager);
projectManager = new ProjectManager(breakpointManager, locationManager);
projectManager.mainProject = new Project({
rootDir: rootDir,
files: [],
Expand All @@ -74,7 +82,7 @@ describe('DebugProtocolAdapter', function() {

afterEach(async () => {
sinon.restore();
client?.destroy();
client?.destroy(true);
//shut down and destroy the server after each test
await server?.stop();
await util.sleep(10);
Expand All @@ -86,6 +94,8 @@ describe('DebugProtocolAdapter', function() {
async function initialize() {
await adapter.connect();
client = adapter['socketDebugger'];
client['options'].shutdownTimeout = 100;
client['options'].exitChannelTimeout = 100;
//disable logging for tests because they clutter the test output
client['logger'].logLevel = 'off';
await Promise.all([
Expand Down Expand Up @@ -140,6 +150,136 @@ describe('DebugProtocolAdapter', function() {
});

describe('syncBreakpoints', () => {
it('retries at next sync() to delete breakpoints if first request failed', async () => {
await initialize();
//disable auto breakpoint verification
client.protocolVersion = '3.2.0';

//add a single breakpoint first and do a diff to lock in the diff
const bp2 = breakpointManager.setBreakpoint(srcPath, { line: 2 });

await breakpointManager.getDiff(projectManager.getAllProjects());

//add breakpoints
const [bp1, bp3] = breakpointManager.replaceBreakpoints(srcPath, [
{ line: 1 },
{ line: 3 }
]);

//sync the breakpoints so they get added
plugin.pushResponse(AddBreakpointsResponse.fromJson({
breakpoints: [{
id: 8,
errorCode: ErrorCode.OK,
ignoreCount: 0
}, {
id: 9,
errorCode: ErrorCode.OK,
ignoreCount: 0
}],
requestId: 1
}));

//sync the breakpoints. this request will fail, so try deleting the breakpoints again later
await adapter.syncBreakpoints();

//now try to delete the breakpoints
breakpointManager.deleteBreakpoints([bp1, bp3]);

//complete request failure because debugger not stopped
plugin.pushResponse(GenericV3Response.fromJson({
errorCode: ErrorCode.NOT_STOPPED,
requestId: 1
}));

//sync the breakpoints again. ask to delete the breakpoints, but it fails.
await adapter.syncBreakpoints();

expect(
[...breakpointManager.failedDeletions.values()].map(x => x.deviceId)
).to.eql([8, 9]);

plugin.pushResponse(RemoveBreakpointsResponse.fromJson({
breakpoints: [{
id: 8,
errorCode: ErrorCode.OK,
ignoreCount: 0
}, {
id: 9,
errorCode: ErrorCode.OK,
ignoreCount: 0
}],
requestId: 1
}));

await adapter.syncBreakpoints();
expect(plugin.getLatestRequest<RemoveBreakpointsRequest>().data.breakpointIds).to.eql([8, 9]);
});

it('removes any newly-added breakpoints that have errors', async () => {
await initialize();

const [bp1, bp2] = breakpointManager.replaceBreakpoints(srcPath, [
{ line: 1 },
{ line: 2 }
]);

plugin.pushResponse(AddBreakpointsResponse.fromJson({
breakpoints: [{
id: 3,
errorCode: ErrorCode.OK,
ignoreCount: 0
}, {
id: 4,
errorCode: ErrorCode.INVALID_ARGS,
ignoreCount: 0
}],
requestId: 1
}));

//sync breakpoints
await adapter.syncBreakpoints();

//the bad breakpoint (id=2) should now be removed
expect(breakpointManager.getBreakpoints([bp1, bp2])).to.eql([bp1]);
});

it('only allows one to run at a time', async () => {
let concurrentCount = 0;
let maxConcurrentCount = 0;

sinon.stub(adapter, '_syncBreakpoints').callsFake(async () => {
console.log('_syncBreakpoints');
concurrentCount++;
maxConcurrentCount = Math.max(0, concurrentCount);
//several nextticks here to give other promises a chance to run
await util.sleep(0);
maxConcurrentCount = Math.max(0, concurrentCount);
await util.sleep(0);
maxConcurrentCount = Math.max(0, concurrentCount);
await util.sleep(0);
maxConcurrentCount = Math.max(0, concurrentCount);
await util.sleep(0);
maxConcurrentCount = Math.max(0, concurrentCount);
concurrentCount--;
});

await Promise.all([
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints(),
adapter.syncBreakpoints()
]);
expect(maxConcurrentCount).to.eql(1);
});

it('removes "added" breakpoints that show up after a breakpoint was already removed', async () => {
const bpId = 123;
const bpLine = 12;
Expand Down Expand Up @@ -188,6 +328,9 @@ describe('DebugProtocolAdapter', function() {
//delete the breakpoint (before we ever got the deviceId from the server)
breakpointManager.replaceBreakpoints(srcPath, []);

//run another breakpoint diff to simulate the breakpoint being deleted before the device responded with the device IDs
await breakpointManager.getDiff(projectManager.getAllProjects());

//sync the breakpoints again, forcing the bp to be fully deleted
let syncPromise = adapter.syncBreakpoints();
//since the breakpoints were deleted before getting deviceIDs, there should be no request sent
Expand Down Expand Up @@ -243,14 +386,14 @@ describe('DebugProtocolAdapter', function() {
//sync the breakpoints to mark this one as "sent to device"
await adapter.syncBreakpoints();

//replace the breakpoints before they were verified
adapter['breakpointManager'].replaceBreakpoints(`${rootDir}/source/main.brs`, []);
breakpoint.deviceId = undefined;
// //replace the breakpoints before they were verified
// adapter['breakpointManager'].replaceBreakpoints(`${rootDir}/source/main.brs`, []);
// breakpoint.deviceId = undefined;

//sync the breakpoints again. Since the breakpoint doesn't have an ID, we shouldn't send any request
await adapter.syncBreakpoints();
// //sync the breakpoints again. Since the breakpoint doesn't have an ID, we shouldn't send any request
// await adapter.syncBreakpoints();

expect(plugin.latestRequest?.constructor.name).not.to.eql(RemoveBreakpointsResponse.name);
// expect(plugin.latestRequest?.constructor.name).not.to.eql(RemoveBreakpointsResponse.name);
});

it('skips sending AddBreakpoints and AddConditionalBreakpoints command when there are no breakpoints', async () => {
Expand Down
Loading

0 comments on commit f1b16f7

Please sign in to comment.