start playing with hat tests #1815

josharian · 2023-08-20T00:14:08Z

for voice discussion. definitely work in progress.

packages/cursorless-engine/src/core/hatStats.test.ts

josharian · 2023-08-20T01:47:47Z

to discuss (notes to self):

general approach
allocation determinism
formatting etc, relation to other draft prs
performance
cross-package imports
better hat representation? (e.g. _ for default, single height for colors, double for both? mixes line meanings, need unambiguous color/shape letters, but better visually??)
which expansions are in scope?

todo (notes to self):

remove normalization routines
work through all todos in code
rename InMemory to Mock?
performance
add other language samples
try out on other hat experiments (including non-buggy first word experiment)
don't forget about logging :)
concise positions, even more concise

pokey

Just left quick comment; haven't really reviewed

pokey · 2023-08-20T13:56:56Z

packages/common/src/testUtil/mockEditor.ts

Oh wow ok. We were planning to just use https://github.com/microsoft/vscode-languageserver-node as we'll need that for lsp anyway 😅. Looks like your code goes a bit further, as they stop at text document and you go all the way to editor. But would be good to use as much of their code as possible as we'll be pulling that in anyway

@pokey I went to pull this in and hit a speed bump. Their TextDocument type implements a smaller interface than the cursorless TextDocument interface:

Type 'TextDocument' is missing the following properties from type 'TextDocument': range, eol, lineAt ts(2739)

Range and eol are trivial, but unfortunately, lineAt returns a non-trivial TextLine interface, so there's a meaningful amount of code missing.

I could try to (a) shrink our interface or (b) build our mock document as a wrapper around theirs, harder because of the TextLine thing, or (c) just continue with our own top-to-bottom mock. Or I guess (d) try to upstream code to make theirs match our needs.

Thoughts?

I'd lean towards a), falling back to b) as necessary

(a) did not go well. We use the missing properties a lot. Half an hour in, and several false victories later...

Moving on to plan b.

Sigh. Plan B isn't promising either. We still need our implementation of TextLine, and re-implementing the rest in terms of their interface is janky. Not done, but the diff-stat isn't overwhelming:

packages/common/src/testUtil/mockEditor.ts | 51 +++++++++++++++++----------------------------------

I think we should either (a) have you take a crack at this or (b) discuss again at a meet-up. Maybe there's something clever we can do that I'm missing...but these two interfaces don't line up nicely.

Moved this discussion to #1882

josharian · 2023-08-22T00:37:30Z

update from meeting: this is promising. i will continue to tinker with it.

josharian · 2023-08-24T01:17:28Z

this is the sort of thing that I could accidentally tinker with forever. there are lots of improvements that can be made and lots of directions to go. but the best possible direction is submitted, where it can be improved incrementally. :)

I know that this cannot go in as it is--it is lacking a fair bit of polish--but I would like to start the review process, because I think it will take several iterations. (btw, this currently includes #1825, because it is ~unusable without it.)

feel free to only do a high level review at this point if you would prefer. over voice would also be okay. up to you.

josharian · 2023-08-24T01:19:40Z

also, if you have any talon, python or typescript files of a medium-ish length, with no licensing issues, and that doesn't pertain directly to cursorless (so that it doesn't constantly appear when we grep the codebase), I love to add them...

josharian · 2023-08-29T17:56:32Z

update from meetup:

we will use fake hat colors and shapes that have really obvious ascii representations. we don't need blue/fox/etc.
overall ascii representation looking promising. we explored a bunch of alternatives, but didn't find one we liked better than this.
josh to try to continue to speed up hat allocation so that the tests don't time out.
josh to hook up update fixture vs test fixture code.

josharian · 2023-09-01T18:23:18Z

pokey · 2023-09-04T12:04:37Z

would love some sample files in other languages, particularly typescript, python, and a LISP-y language (or something else that is punctuation-heavy). see start playing with hat tests #1815 (comment)

chatgpt? 😄 Happy to add some tho if you prefer

are you happy with how this is triggered? (update fixtures to update, tests fixtures during unit tests)

yep makes sense

is there a way to avoid bringing up the extension window during "update fixtures subset" if none of the tests in the subset needed?

not sure if this is what you're after, but we should have a launch config that updates fixtures only for the unit tests. should be trivial to add. and that won't pop up the extension window at all I believe

pokey · 2023-09-04T11:51:04Z

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.golden

@@ -0,0 +1,159 @@
+_  _       _            _      _ _
+// Package intern interns strings.
+[] [-----] [----] [-----] [-----]|


I think I like this convention

pokey · 2023-09-04T11:52:11Z

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.golden

+func String(s string) string {
+[--] [----]|| [----]| [----] |
+
+ B AA  B  1 B CC2D B A3     A4     D


What do all these letters / numbers mean? I guess they're colors / shapes. And colored shape uses two rows?

I added detail to the readme

pokey · 2023-09-04T11:52:25Z

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.golden

where's the cursor? always at 0:0?

added to read me

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.golden

pokey · 2023-09-04T11:53:13Z

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.stats

+	▁▁▁▁▁▁▃▁▃▁▁▁▃▃▁▁▁▃▁▁▃▁▁▃▁▁▃▃▃▁▁▃▃▁▃▁▃▃▃▃▃▁▃▃▁▃▃▃▃▃▃▃▁▃▃▃▁▃▁▃
+	▁▁▃▃▁▃▃▃▃▃▁▃▁▃▃▃▁▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃
+	▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▅▃▃▃▃▃▃▃▅▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▅▃▃▃▃▃
+	▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▃▅▃▃▃▃▃▃▃▅▃▃▃▃▃▃▃▃▅▃▃▅▃▅▃▅▃▅▃▅▃


how am I supposed to read this chart? Is the x tokens, and the y penalty?

added to read me, please let me know if it is insufficient

pokey · 2023-09-04T11:53:43Z

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.stats

+nHats:
+	mean: 100%
+	std: 0%
+	min: 100%
+	max: 100%
+	spark: 0              25               50               75             █ 100


what do these statistics mean?

added to read me

pokey · 2023-09-04T11:53:58Z

packages/cursorless-engine/src/test/fixtures/hat-stats/intern.go.stats

+	std: 1%
+	min: 80%
+	max: 82%
+	spark: 0              25               50               75  ▂█▆          100


what's this funny shape?

added to read me, might not be sufficient, please let me know if you still have questions

josharian · 2023-09-07T02:27:24Z

chatgpt? 😄 Happy to add some tho if you prefer

I'd rather use human written code. I'll grab some from a friend if you don't have some handy.

we should have a launch config that updates fixtures only for the unit tests

done

josharian · 2023-09-07T02:28:20Z

i have sinned...force pushed. sorry. will switch back to regular review mode now.

this is interesting because of the low number of nPenalty0 hats.

auscompgeek · 2023-09-07T09:42:48Z

.gitattributes

@@ -0,0 +1,3 @@
+# do not modify line endings of our hat test golden files
+packages/cursorless-engine/src/test/fixtures/hat-stats/*.golden -crlf


I thought the attribute is called eol=lf, or -text for "don't mangle this at all"

Oh no not this again 😅. What's the issue you're trying to solve here @josharian?

It’s solved now, thanks to @auscompgeek .

The issue was that git was adding CR to the golden files on checkout, so the test that the golden files were unmodified was failing.

I would prefer to avoid gitattributes if at all possible. If people want to check things out as CRLF, I don't see why we should get in the way of that. The last time we ran into this problem, we solved it by normalizing when we read the gold file. See

cursorless/packages/cursorless-vscode-e2e/src/suite/testCaseRecorder.vscode.test.ts

Lines 169 to 171 in 5789094

// We use this to ensure that the test works on Windows. Depending on user

// / CI git config, the file might be checked out with CRLF line endings

.replaceAll("\r\n", "\n");

OK. If you look at the commit history, I’ve also implemented that, when you gitattributes originally did not work. Although to avoid needless diffs, we also have to normalize at writing time as well…sigh.

This reverts commit f4bd698.

josharian · 2023-09-11T02:24:25Z

marking as draft until #1882 is sorted

- Extracted from #1815 ## Checklist - [-] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [-] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [-] I have not broken the cheatsheet

- Extracted from cursorless-dev#1815 ## Checklist - [-] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [-] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [-] I have not broken the cheatsheet

github-advanced-security bot found potential problems Aug 20, 2023

View reviewed changes

packages/cursorless-engine/src/core/hatStats.test.ts Fixed Show fixed Hide fixed

josharian force-pushed the josh/hat-stat-tests branch 2 times, most recently from 8610ad1 to f8770ab Compare August 20, 2023 01:46

josharian added the to discuss Plan to discuss at meet-up label Aug 20, 2023

josharian force-pushed the josh/hat-stat-tests branch 2 times, most recently from 1190188 to 9179428 Compare August 20, 2023 05:18

pokey reviewed Aug 20, 2023

View reviewed changes

josharian removed the to discuss Plan to discuss at meet-up label Aug 22, 2023

josharian force-pushed the josh/hat-stat-tests branch from 1857eb7 to 59206d4 Compare August 24, 2023 01:09

josharian marked this pull request as ready for review August 24, 2023 01:15

josharian force-pushed the josh/hat-stat-tests branch from 8e8d3a1 to c9213b3 Compare September 1, 2023 02:34

josharian mentioned this pull request Sep 1, 2023

memoize minimumTokenRankContainingGrapheme #1825

Merged

pokey reviewed Sep 4, 2023

View reviewed changes

josharian added 3 commits September 6, 2023 11:33

start on hat stats and golden tests

26ae40a

improve hat-stats readme

a88f79d

add launch task to update fixtures for unit tests only

53a07b8

josharian force-pushed the josh/hat-stat-tests branch from 4e8fefd to 53a07b8 Compare September 7, 2023 02:27

josharian added 3 commits September 6, 2023 19:32

add gitattributes to fix crlf issues on windows (i hope)

c377196

add elixir sample from river (thanks!)

6ca0319

this is interesting because of the low number of nPenalty0 hats.

try fixing \r\n a different way

f4bd698

auscompgeek reviewed Sep 7, 2023

View reviewed changes

josharian added 4 commits September 9, 2023 17:07

add missing crlf

250b404

Revert "try fixing \r\n a different way"

98ade7e

This reverts commit f4bd698.

start on token score equivalences

4b4f894

try -text

4b3186b

josharian mentioned this pull request Sep 10, 2023

add in-memory TextDocument implementation #1882

Closed

josharian marked this pull request as draft September 11, 2023 02:24

pokey mentioned this pull request Oct 17, 2023

Add launch config to update unit test fixtures #1956

Merged

pokey added the blocked Blocked on something; eg another PR being completed. Look in comments of issue / PR for reason label Jun 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

start playing with hat tests #1815

start playing with hat tests #1815

josharian commented Aug 20, 2023

josharian commented Aug 20, 2023 •

edited

Loading

pokey left a comment

pokey Aug 20, 2023

josharian Aug 22, 2023

pokey Sep 3, 2023

josharian Sep 6, 2023 •

edited

Loading

josharian Sep 7, 2023

josharian Sep 11, 2023

josharian commented Aug 22, 2023

josharian commented Aug 24, 2023 •

edited

Loading

josharian commented Aug 24, 2023

josharian commented Aug 29, 2023 •

edited

Loading

josharian commented Sep 1, 2023 •

edited

Loading

pokey commented Sep 4, 2023 •

edited

Loading

pokey Sep 4, 2023

pokey Sep 4, 2023

josharian Sep 7, 2023

pokey Sep 4, 2023

josharian Sep 7, 2023

pokey Sep 4, 2023

josharian Sep 7, 2023

pokey Sep 4, 2023

josharian Sep 7, 2023

pokey Sep 4, 2023

josharian Sep 7, 2023

josharian commented Sep 7, 2023

josharian commented Sep 7, 2023

auscompgeek Sep 7, 2023

pokey Sep 10, 2023

josharian Sep 10, 2023

pokey Sep 10, 2023

josharian Sep 10, 2023

josharian commented Sep 11, 2023

		@@ -0,0 +1,3 @@
		# do not modify line endings of our hat test golden files
		packages/cursorless-engine/src/test/fixtures/hat-stats/*.golden -crlf

	// We use this to ensure that the test works on Windows. Depending on user
	// / CI git config, the file might be checked out with CRLF line endings
	.replaceAll("\r\n", "\n");

start playing with hat tests #1815

Are you sure you want to change the base?

start playing with hat tests #1815

Conversation

josharian commented Aug 20, 2023

josharian commented Aug 20, 2023 • edited Loading

pokey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josharian Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josharian commented Aug 22, 2023

josharian commented Aug 24, 2023 • edited Loading

josharian commented Aug 24, 2023

josharian commented Aug 29, 2023 • edited Loading

josharian commented Sep 1, 2023 • edited Loading

early reviewer input requested

my next steps

pokey commented Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josharian commented Sep 7, 2023

josharian commented Sep 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josharian commented Sep 11, 2023

josharian commented Aug 20, 2023 •

edited

Loading

josharian Sep 6, 2023 •

edited

Loading

josharian commented Aug 24, 2023 •

edited

Loading

josharian commented Aug 29, 2023 •

edited

Loading

josharian commented Sep 1, 2023 •

edited

Loading

pokey commented Sep 4, 2023 •

edited

Loading