Skip to content

Conversation

@belcar-s
Copy link
Collaborator

@belcar-s belcar-s commented Jun 18, 2022

This pull request is based on my two other branches: minimal-changes and excessive-reloading. I can rebase it if those changes are rejected. If they aren’t rejected, though, doing so is not a good idea; it would involve tons of conflict resolution (I think?) that would later need to be reversed. (The commit history and diffs in this page would supposedly be updated if the other pull requests were merged.)

Like last time, icons are missing. We need a test_sidebar_large, a test_sidebar_small and a run. The run is for the Run All button. I think an icon is better than a label, but we can use a label if that's preferred :).

Demonstration.mp4

The above video showcases an older version of the feature. In the current version, a warning is shown the first time the user runs tests in each workspace. The notification that is displayed after three seconds of the tests running could become annoying; it would be good to allow it to be disabled.

Changes unique to this pull request begin at Define the sidebar.

@belcar-s belcar-s marked this pull request as draft June 18, 2022 00:14
@belcar-s belcar-s force-pushed the test-sidebar branch 3 times, most recently from 913ac55 to d922e06 Compare June 19, 2022 04:46
@belcar-s
Copy link
Collaborator Author

belcar-s commented Jun 19, 2022

warning symbol

I don't know how to use this RegExp, which I found in the doowb/ansi-colors source code, without committing copyright infringement/becoming a criminal, and getting sent to jail.

/[\u001b\u009b][[\]#;?()]*(?:(?:(?:[^\W_]*;?[^\W_]*)\u0007)|(?:(?:[0-9]{1,4}(;[0-9]{0,4})*)?[~0-9=<>cf-nqrtyA-PRZ]))/g

It's seemingly licensed MIT. Do I add the thing to the LICENSE file, or is it OK to add the disclaimer next to the copied line? Sorry 🙁.

@belcar-s belcar-s force-pushed the test-sidebar branch 3 times, most recently from e77fbc6 to bf3644b Compare June 19, 2022 21:07
@belcar-s
Copy link
Collaborator Author

belcar-s commented Jun 19, 2022

Wow! This is nearly done. I think I overestimated the difficulty. (Watch me turn out incapable of doing the rest 🙂.)

Note that we currently obtain the test results by parsing the logging produced by Deno, which must be prone to change. I'd love to know of a better way to do this.

Things remaining:

  • add the needed icons
  • enable running of a single test with context menus
  • respect the import map thing workspace preference
  • automatically reload the tree view when files are added or removed
  • try to stop the files' order from changing when tests are run
  • pick prettier colors (more or less)
  • fix the bug that's apparently impeding dolor.test.ts's outcomes from appearing (.test.ts is empty and this is in reference to the video)
  • remove the arrow to the left of files whose outcomes aren't stored

Ideas:

  • disambiguation, like in the symbols sidebar
  • enable double-clicking to view a test's definition (This seems a little bit hard.)
  • showing test results as they are obtained
  • enable double-clicking to open a test file (This is much easier :))

Copy link
Collaborator Author

@belcar-s belcar-s left a comment

Choose a reason for hiding this comment

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

This is a really cool change because —together with the refactor, I assume —it adds a helpful, beautiful new placeholder to the symbols sidebar. Finally! I'm certain I tried this when I first wrote it; it might depend on the tree view being immediately registered. (Before, it wasn't registered until the Find Symbol command was used.)

placeholder

I know that I'm claiming this pull request is about the tests. It's a little bit hard for me to propose these changes separately, but I'll try my best from now on.

@belcar-s belcar-s force-pushed the test-sidebar branch 2 times, most recently from 52421e3 to b67ac0e Compare June 21, 2022 01:40
@belcar-s

This comment was marked as outdated.

@belcar-s belcar-s force-pushed the test-sidebar branch 8 times, most recently from 803449f to 048f9f0 Compare June 23, 2022 20:54
@belcar-s
Copy link
Collaborator Author

belcar-s commented Jun 23, 2022

It might have been a bad idea to squash the changes from the other branches into 247786b because I think that the commit will be kept now. I don't know how to undo that 🙁.

Outdated; this was fixed in 7143d46

I've also been seeing these error messages:

An error message reads 'Extension Error. The extension Deno encountered an error the last time Nova was open. Do you want to disable it?  You can enable it again in the Extensions preferences. An error message has the title 'Extension Service Quit Unexpectedly'. Below, three paragraphs read 'An issue with one or more extensions caused the service to terminate. As a result, all extensions will need to reload. The following extensions were executing when the crash occurred: • Deno (co.gwil.deno) You may optionally disable extensions until the next application launch.'

It's a little bit bad. I haven't been able to find any logging about the issue, so it's hard to debug. It happens immediately after activation and only in workspaces that aren't associated to a folder (those that appear when you press ⇧⌘N).

In regard to the hidden section above, see https://devforum.nova.app/t/a-fun-new-way-to-crash-the-extension-service/1682.

@belcar-s belcar-s force-pushed the test-sidebar branch 4 times, most recently from 64289c1 to 2c12805 Compare June 25, 2022 23:18
@belcar-s belcar-s marked this pull request as ready for review June 25, 2022 23:20
@belcar-s belcar-s marked this pull request as draft June 27, 2022 22:14
@belcar-s
Copy link
Collaborator Author

I'm going to try to make it show test results as they come, and I'll make it notify that tests couldn't be run even if the attempt was made through a context menu.

@belcar-s
Copy link
Collaborator Author

The particular change I made in order to enable the first of those two things made test-running much slower, and I'm not really so enthusiastic about trying again, so I'll leave that as it is 🙃.

@belcar-s belcar-s marked this pull request as ready for review June 29, 2022 00:44
Reload symbols only after their order or names are changed

Keep the header message accurate

Supposedly improve type checking

Keep a record that enables access to a symbol's updated location using its ID

Remove unneeded properties

Do not ignore all errors

Make `watchConfigFile` function less exceptional

It now takes an array of filenames and returns an array of disposables. It was also placed closer to `restartServerOnConfigChanges` and `restartServerOnWorkspaceConfigChanges`.

Use the new `configFilenames` constant

More concisely remove item from array
Put placeholder text field in the right location

Provide an explanation of the refresh button
This commit includes a change to the `LICENSE` file, which may be undesired.

Add indicators of whether each test passed

I can't get identifiers to work 🙃

Don't show confusing expansion arrow

Document mysterious code

Correctly handle files with a single test

Also, notify the user of empty files.

Try to use nicer colors

Watch file system

Respect unstable API and import map preferences

Don't replace sidebar after failure to parse

Enable a double-click to be performed to open a test file

Disable the Run button in test context menus

I don't know whether the second change is necessary (`TestsDataProvider.ts`).
This includes highly precise measuring of time, I think, as well as loading dynamic libraries.

Remove VS Code settings folder

I swear I'm not a traitor! I just use the thing sometimes 😊.

Remove aosdijfoaisjdf.test.ts

This is what I get for git commit -a…ing.
Update message for accuracy

Remove debug logging

Remove unnecessary logging

Suggest to users that Deno may be failing to run tests

Provide feedback even when tests take long
I don't know how much better this is.
The message couldn't really be read and I think the placeholder defined in the `extension.json` file is appropriate.
This might be super broken, but so is my brain right now. For mysterious and supposedly unrelated reasons, I'm incredibly tired! I'm trusting that this code is useful even if it needs improvements. A lot of it assumes that the `path` property of corresponding `TestFile`s will always match, but that might not happen if certain algorithm changes are made, the consequences of which would not be able to be anticipated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants