aspnet / dnx

OBSOLETE - see readme
Other
963 stars 224 forks source link

dnx --watch not working #1795

Closed CoreyKaylor closed 9 years ago

CoreyKaylor commented 9 years ago

I have tried a few different combinations, but I believe the --watch option for dnx isn't working as advertised. The following dnx --watch . test will run the tests, but will not monitor for changes. I'm running this on Windows, powershell, beta4. Is it possible my usage is just wrong?

akamud commented 9 years ago

We are discussing this here aspnet/Hosting#45, it looks like it does watch for changes (as it is killed if you change a .cs file), but it doesn't restart the process.

CoreyKaylor commented 9 years ago

In my case that isn't true. There is no process left running that's monitoring for changes. It runs the tests once and that's it.

akamud commented 9 years ago

Sorry @CoreyKaylor, I think what I said was misleading. This is my scenario right now (on Windows):

dnx . --watch web Change a .cs file, the process is stopped. This is confirmed here
The problem is that it doesn't start up again.

To illustrate the difference, if I just run dnx . web and I change a .cs file, it doesn't kill the process, it is still running. So --watch is indeed watching for changes, it just isn't doing the work completely as we expect.

muratg commented 9 years ago

@Tratcher

Tratcher commented 9 years ago

@CoreyKaylor The test command is designed to exit after executing the tests once, I don't think --watch is ever going to make sense for tests.

The web command however is designed to run until requested to terminate. --watch monitors for changes and kills the process. We're still discussing who should restart it.

CoreyKaylor commented 9 years ago

It's hard to hear such an absolute statement like "I don't think --watch is ever going to make sense for tests" without responding in a snarky fashion so forgive me. The statement alone implies that --watch shouldn't belong on dnx in that case. Without it there, it may have to require config changes? This would be less than ideal for most users looking for this behavior.

I can see that when implementing a --watch capability with a very narrow minded view to support live reload capabilities for web application development that you could have that opinion. It absolutely is useful for tests to provide auto test capabilities. Almost every community in existence can do this today so I would say that alone validates whether or not it makes sense, but I'm not sure why I should need to defend that point. Good news is that regardless of the outcome nodemon works fine to achieve the behavior I'm looking for.

To the statment about terminate after test run, or terminate after requested to do so. What difference does that make? The same behavior is possible regardless. To a user executing dnx and seeing the list of options, one of which is --watch the expectation is the same. The process will restart after a change is made automatically giving a fast feedback cycle that doesn't require me ever leaving my editor.

akamud commented 9 years ago

I agree that --watch is very confusing right now. We expect the same behavior from all the other tools we are used to work with every day.

You can see by the number of issues and Jabbr questions regarding this, if watch the way it is today is the intended goal, we must somehow address this common misunderstanding.

CoreyKaylor commented 9 years ago

Yeah, it's a bit naive to incorporate watch from the perspective of a single project as well. Doesn't really account for dependent project references without deeper analysis that's probably overkill anyways. Watch should almost always be from the root of the repository in my opinion.

CoreyKaylor commented 9 years ago

For anyone else looking for similar behavior this works great. From the root of your repository you can run the following.

nodemon --ext "cs,json" --exec "dnx ./tests/pathtotestprojectdir test"

mikeobrien commented 9 years ago

@Tratcher "I don't think --watch is ever going to make sense for tests." This is the kind of comment that makes us look bad to other communities. Everybody else has this and it rocks.

Tratcher commented 9 years ago

@CoreyKaylor @mikeobrien That's a usage I've never heard of. You could probably make it work. Xunit currently exits at the end of every test run, so would your nodemon sample run the tests continuously in a loop even if there weren't changes? Our does it ignore the app self-termination and just re-run the command when it detects changes?

CoreyKaylor commented 9 years ago

No it doesn't run them in a continuous loop even when there are no changes. It's the equivalent of saying every time you detect changes, run this Process.Start that I hand you in --exec option and to your point the test project exits and nodemon reports in the first run the output from the test execution as well as the exit status of the test process. You make a change and it re-runs the process again. If there are failed tests, failed compile, etc. I never leave my editor and get feedback typically in under a second including the time for it to detect that a file changed.

To bring it back to the current supported use-case. Most other ways I've seen the implementation for --watch in web applications is a proxy server is started and proxies requests to the application. In the case of a file change, the proxy application ensures that the real application process has been restarted, but the user's request from the browser never dies because it's interacting with the proxy and not the application itself. This also makes it easier in most cases to show more contextual startup problems because the proxy can report those errors in a web response as well. To your earlier comment about the process exit, the --watch option if selected should deal with process crashes as well. So once the code responsible for the crash is fixed it can begin serving requests again without restarting your command that technically should have continued to watch and restart.

Tratcher commented 9 years ago

I reviewed this with the original feature designers / implementers and verified that everything is working per the original design. That said, clearly the design does not live up to user expectations and we have more work to do.

Notes:

CoreyKaylor commented 9 years ago

Still also think the watch needs to expand beyond the view of the project only. It's very common to change a file in a referenced project within the same solution. It would be ideal to not require re-running the command in that case.

davidfowl commented 9 years ago

That already works. Any files used in the compilation are watched.

CoreyKaylor commented 9 years ago

That's good to hear. That part was an assumption on my part, and as assumptions go...

cherrydev commented 9 years ago

I can't get --watch to work at all using 'test'.

I figured I'd be clever and make a batch script that re-runs dnx --watch . test -wait. I assumed that adding --watch would kill the process while it was waiting for user input and then the bash script would re-run it. Unfortunately, changing a file even in the CWD project doesn't kill the test process under this circumstance.

(It does work using Kestrel, even on a dependent project.)

CoreyKaylor commented 9 years ago

@cherrydev my comment regarding nodemon workaround for now has been working well for me.

cherrydev commented 9 years ago

Yes, I saw it and it does work, but having it monitor the actual actively executing source code would be even better!

davidfowl commented 9 years ago

The file watcher on mono is sorta busted.

cherrydev commented 9 years ago

Should have been specific: This was on Windows.

dpnevmatikos commented 9 years ago

The file watcher on mono is sorta busted.

@davidfowl I also can't get it to work properly (Linux mint) using: dnx --watch . kestrel

It kills the process completely...

cherrydev commented 9 years ago

@dpnevmatikos if you read the rest of the above thread, you'll see that killing the process is the currently expected behaviour.

johnpapa commented 9 years ago

I liked this so I put this together for OSX as a bash script

https://github.com/johnpapa/aspnet5-starter-demo#dnxmon

Tragetaschen commented 9 years ago

I think there is one piece missing in this discussion: The file watching implementation requires the running process to cooperate to actually shutdown. It's using the IApplicationShutdown infrastructure and signals the ShutdownRequested CancellationToken, so if the running application does not set itself up to do anything with this, it will look like watching isn't working.

Microsoft.AspNet.Hosting uses the cancellation token as the one thing to end itself, so watching the typical web command works. The test runner on the other hand doesn't do anything with it, so tests will always run in their entirety and exit at the end, while fully ignoring any intermediate request.

Tratcher commented 9 years ago

Recommendation: Have a separate tool (global command) to monitor the process and restart it if it dies. This may require a contract between the process and watchdog for exit codes.

We at least need to provide viable samples for this scenario.

Buildstarted commented 9 years ago

Here's a simple way to restart dnx with a batch file on windows

for /l %%x in (1, 1, 1000000) do (
    dnx --watch . web
)
davidfowl commented 9 years ago

Closing this out as it's tracked by https://github.com/aspnet/Hosting/issues/344