Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spin watch doesn't rebuild/reconfigure on manifest file changes #2480

Closed
garikAsplund opened this issue Apr 30, 2024 · 6 comments · Fixed by #2481
Closed

spin watch doesn't rebuild/reconfigure on manifest file changes #2480

garikAsplund opened this issue Apr 30, 2024 · 6 comments · Fixed by #2481

Comments

@garikAsplund
Copy link
Contributor

garikAsplund commented Apr 30, 2024

Expected behavior:

Manifest file changes must trigger a full rebuild and restart, and must reload the watch configuration (because we may now be watching different paths). This is currently managed by the ManifestFilterFactory, Reconfiguriser and ReconfigurableWatcher.

Originally posted by @itowlson in #2478 (comment)

Making changes within spin.toml while spin watch is running does not lead to any rebuild or reconfigurations.

@itowlson
Copy link
Contributor

Looks like it's unhappy that the manifest path is absolute. I'm not sure if this is a watchexec change, or if we changed Spin so that we absolutised the manifest path before giving it to watch. But it looks like changing the manifest path to be relative to the manifest directory fixes it.

@itowlson
Copy link
Contributor

@garikAsplund I saw you had 👀 on the original comment - does that mean you're interested in investigating and fixing this? I'm happy to assign it to you if you want, otherwise I'll pick it up. (I'm not trying to thrust it on you - just don't want to tread on your toes.)

@garikAsplund
Copy link
Contributor Author

@itowlson If it's quick and easy for you to fix go right ahead! Or I can investigate further tonight if you're busy.

@itowlson
Copy link
Contributor

Yeah I can sort it. Thanks for spotting this!

Context: #1983 (so we do need the absolute but not sure exactly when)

@garikAsplund
Copy link
Contributor Author

Cool! Appreciate the context.

Out of curiosity, what's the best way of testing CLIs, especially with file changes and everything?

@itowlson itowlson linked a pull request Apr 30, 2024 that will close this issue
@itowlson
Copy link
Contributor

I have yet to find a method better than manual testing for cases like this, although with Ryan's integration/runtime test framework we might be in a better position to automate something. "UI tests" (which store an expected "golden" output and diff the actual output against it, hence also known as "golden tests") can help with some CLI tests - but that wouldn't really help here.

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 a pull request may close this issue.

2 participants