sg-wireless / pymakr-vsc

GNU General Public License v3.0
97 stars 25 forks source link

Next jv #226

Closed Josverl closed 2 years ago

Josverl commented 2 years ago

Addressed a few items

Josverl commented 2 years ago

I have a hard time with validation of the above and more the changes for windows paths as the integration tests are not working on windows, and debugging them is complicated by :

I understand that there a groups of tests that make most/only sense when run in order, but currently there is only one possible path for all integration tests, and that currently blocks me in in validating (possible) fixes as there are multiple tests that fail to run in my setup that seem to interfere with the other tests.

I would welcome a framework such as mocha or jest are there are more than a few samples to make vscode extension testing work , and also work in GHA pipelines which would help catch cross-platform bugs.

Josverl commented 2 years ago

Possibly the integration repl test fail as a result of the the terminals not being closed correctly at the end of a test ( or at all ) the clue is that multiple terminals to the same device are showing when running the integration test twice :

another possibility is that the response to the test command is sent / received by the host in multiple chunks, and that the function nextTerminalData only processes the first chunk received.

image

❌ can use print command  738 ms
node_modules/probs/lib/reporters/consoleReporter/consoleReporter.js:11
AssertionError: 
- Expected
+ Received

  print("foo")
- foo
- >>> 
+
    at Object.<anonymous> (/home/jos/pymakr-vsc/test/suite/integration/devices.test.js:44:16)
    at async [file:///home/jos/pymakr-vsc/node_modules/probs/lib/test.js:41:13]()
    at async processQueue ([file:///home/jos/pymakr-vsc/node_modules/probs/lib/utils/misc.js:57:32]())
jakobrosenberg commented 2 years ago

Thanks for this PR @Josverl. I skimmed it and it look great. I'll review it ASAP and get back to you.

As for testing: I wanted to use the default testing tools (Mocha IIRC), alas I ran into countless issues with integration testing. Eventually after a few days I gave up and installed Probs, which just worked.

The main problem is that testing frameworks are limited to child processes or workers for running tests. This means that they can't access the extension instance directly. It's the same reason that integration tests have to be run from the debugger tab and not the terminal.

With Probs I can use --runner main and have the tests run in the main thread which has access to live extension API. (Granted that I run it under the debugger)

Tests are run in the order that they're written, eg.:

// tests listed by their execution order
test("1", () => {
  test("2", () => {
    test("3", () => {
      test("4", () => {});
      test("5", () => {});
    });
  });
  test("6", () => {});
  test("7", () => {});
});

This makes dependencies easy to reason about, but you're right about the lack of documentation. I'll see if I can do something about that.

FWIW, I tried your branch and all tests, incl. upload, passed. 🥳

jakobrosenberg commented 2 years ago

Great Scott! I think I got integration tests working outside the debugger.

I'll merge this PR and then create a new one to get command line support for the integration tests.

jakobrosenberg commented 2 years ago

:tada: This PR is included in version 2.8.7 :tada:

The release is available on:

Your semantic-release bot :package::rocket: