There's no practical utility in this unless I want to make the palette size variable,
and this makes performance worse updating the screen when the palette changes,
but I'm on a mission to remove `from textual_paint.paint import PaintApp`
since it can't be at top level due to cyclic imports, and it has
side effects when importing `textual_paint.args` in turn.
My TODO comment said "add an attribute to ToolPreviewUpdate or make it's x/y Optional", which is both bad grammar ("it's" vs "its") and a bad idea, since the Canvas isn't involved in this update, so its event definition shouldn't have to be complicated by it.
I like this much better, just splitting the event handler into two functions, giving room to express the optional nature of the coordinate in the signature, and avoiding constructing two message objects at both of the callsites.
If git blame tools were smarter/easier to use, or if I was using a node-based programming paradigm, I would've never made this mistake. I would have done the natural and correct thing in the first place, but instead, I tried to avoid indenting a large block of code, which generates a noisy commit, and a barrier when using git blame.
- The app's directory structure is not a constant, and shouldn't play into this test. Aside from codebase restructuring, directories like `__pycache__` can come and go.
- Even if a temporary directory were created with files enough to fill the view, the scrollbar would still change based on the folder structure outside of the temporary folder.
- pyfakefs is one way to ensure a consistent view of a folder structure for testing. It allows adding real folders in a readonly way. It's more complicated than I thought it would be going in, since I had to add workarounds for pyfiglet and pytest-textual-snapshot, and handle an edge case in my EnhancedDirectoryTree (which got an error which seems to be swallowed?), not to mention pyfakefs raises an error saying "No such file or directory in the fake filesystem" when actually it's the real directory not existing when trying to add it to the fake filesystem, and VS Code was hiding stack frames and refusing to step into library code, and it turned out that I was resolving the absolute path wrong, but it looked right to me because the only part that was missing was "textual-paint", when, at a glance it seemed present, since the "textual_paint" part was present. Ay-ay-ay!
- I don't know if this will fix the problem I saw where these tests' snapshots all changed with no visual or even structural changes, just the IDs of elements changing. I don't know what caused that.
Oh yeah and this is still actually a problem:
============================================= short test summary info ==============================================
FAILED tests/test_snapshots.py::test_paint_open_dialog[light_unicode] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_open_dialog[dark_unicode] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_open_dialog[light_ascii] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_open_dialog[dark_ascii] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_save_dialog[light_unicode] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_save_dialog[dark_unicode] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_save_dialog[light_ascii] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
FAILED tests/test_snapshots.py::test_paint_save_dialog[dark_ascii] - AttributeError: 'EnhancedDirectoryTree' object has no attribute '_id'. Did you mean: 'id'?
========================== 8 failed, 56 passed, 1 xfailed, 1 warning in 152.86s (0:02:32) ==========================
It worked when running in debug, but not when running normally.
Make version info static when running in pytest.
I also tried adding in test_snapshots.py:
import textual_paint
textual_paint.__version__ = "snapshot test edition 1"
import textual_paint.__init__ as init
init.__version__ = "snapshot test edition 2"
which seemed to have no effect.
Since I already have special case logic for __version__ in __init__.py,
I'm reasonably happy with this solution.
I already fixed my first bug caught by the snapshot testing!
These variables were intended as constants, but were being mutated.
I recall writing it as `prefix = (...).stylize_before(...)` and then
moving it to a new line when I realized it was mutation-style method,
not so much the chaining-style factory that I wanted, but I conceived
of it too much as a stylistic distinction in the moment, looking back.
Mutation style means mutation!
Side note: tests also showed a spurious change of a cursor blinking.
I don't really know whether that's in this changeset or not, because
the workflow involves re-running the tests to update the baseline, and
the nice visual diffs provided in the snapshot report aren't available
when viewing the commit diff.
1. If the SVGs were separate files, I could see the diffs on GitHub
or in GitHub Desktop, and maybe some other Git clients.
It would also make it a lot easier to simply view the baselines,
which is useful in general.
2. It would be nice if built-in components didn't cause spurious diffs,
including the Input's cursor blinking and the Header's clock ticking.
I already removed the clock from my gallery app, because it's a sort
of trivial decision, but Inputs I'll have to reckon with.
First I tried setting PYTEST_TEXTUAL_PAINT_ARGS as an environment variable, to be interpreted by args.py, but it turns out args.py is only executed once, not once per test. It's not using subprocesses, only importing and reimporting the app code, and instantiating new App instances, so parts of the code that are at the top level of modules is only evaluated once.
So I found a new strategy, of importing the `args` object in the test fixture and modifying it directly.
I also realized the --ascii-only option permanently modifies Textual's widgets and borders, and my own widgets, for the life of the process, so I'm holding off on that one. I should be able to make --ascii-only mode more dynamic, and could even target it as a runtime toggle, as a goal, since that's basically what I'll need to achieve to get it working for the tests, but thinking of it as a feature is more fun.
I used a very dangerous extension that has a lot of include/exclude
options that are unclear how they interact, and it doesn't apparently
respect your gitignore settings, and it has no no preview or warning.
"commandOnAllFiles.excludeFolders": ["node_modules", "out", ".vscode-test", "media", ".git", ".history", ".venv", ".venv*", ".*"],
"commandOnAllFiles.commands": {
"Organize Imports": {
"command": "editor.action.organizeImports",
"includeFileExtensions": [".py"],
"includeFolders": ["src"]
}
},
