-
Notifications
You must be signed in to change notification settings - Fork 83
feat: Add off mode to UI5_WATCH_MODE
#1565
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,6 +87,11 @@ test.beforeEach(async (t) => { | |
| // Definition-watcher namespace the handler injects into server.serve(). | ||
| t.context.projectWatcher = {default: {create: sinon.stub()}}; | ||
|
|
||
| // Watch-mode facade from @ui5/project. Defaults to watching enabled. Off-mode tests override it. | ||
| t.context.fileWatcher = { | ||
| isWatchingDisabled: sinon.stub().returns(false) | ||
| }; | ||
|
|
||
| // Capture stray writes to stderr/stdout so failing assertions surface the | ||
| // actual output instead of ava's timeout diagnostics. | ||
| t.context.consoleOutput = ""; | ||
|
|
@@ -106,6 +111,7 @@ test.beforeEach(async (t) => { | |
| "@ui5/server/internal/sslUtil": t.context.sslUtil, | ||
| "@ui5/project/graph": t.context.graph, | ||
| "@ui5/project/internal/graph/ProjectDefinitionWatcher": t.context.projectWatcher, | ||
| "@ui5/project/internal/build/helpers/fileWatcher": t.context.fileWatcher, | ||
| "open": t.context.open | ||
| }, { | ||
| "../../../../lib/dataDir.js": { | ||
|
|
@@ -574,6 +580,47 @@ test.serial("ui5 serve --live-reload overrides ui5.yaml liveReload setting", asy | |
| t.is(server.serve.getCall(0).args[1].liveReload, true); | ||
| }); | ||
|
|
||
| test.serial("ui5 serve UI5_WATCH_MODE=off disables live reload (default)", async (t) => { | ||
| const {argv, serve, server, fileWatcher} = t.context; | ||
|
|
||
| fileWatcher.isWatchingDisabled.returns(true); | ||
|
|
||
| serve.handler(argv); | ||
| await t.context.handlerReady; | ||
|
|
||
| t.is(server.serve.callCount, 1); | ||
| t.is(server.serve.getCall(0).args[1].liveReload, false, | ||
| "live reload is forced off when watching is disabled"); | ||
| }); | ||
|
|
||
| test.serial("ui5 serve UI5_WATCH_MODE=off overrides --live-reload", async (t) => { | ||
| const {argv, serve, server, fileWatcher} = t.context; | ||
|
|
||
| argv.liveReload = true; | ||
| fileWatcher.isWatchingDisabled.returns(true); | ||
|
|
||
| serve.handler(argv); | ||
| await t.context.handlerReady; | ||
|
|
||
| t.is(server.serve.callCount, 1); | ||
| t.is(server.serve.getCall(0).args[1].liveReload, false, | ||
| "off wins over an explicit --live-reload"); | ||
| }); | ||
|
|
||
| test.serial("ui5 serve UI5_WATCH_MODE=off overrides ui5.yaml liveReload=true setting", async (t) => { | ||
| const {argv, serve, server, fileWatcher, getServerSettings} = t.context; | ||
|
|
||
| getServerSettings.returns({liveReload: true}); | ||
| fileWatcher.isWatchingDisabled.returns(true); | ||
|
|
||
| serve.handler(argv); | ||
| await t.context.handlerReady; | ||
|
|
||
| t.is(server.serve.callCount, 1); | ||
| t.is(server.serve.getCall(0).args[1].liveReload, false, | ||
| "off wins over server.settings.liveReload"); | ||
|
Comment on lines
+619
to
+621
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like my previous comment. To me it seems that we must test here whether a log message for the liveReload override is displayed |
||
| }); | ||
|
|
||
| test.serial("ui5 serve --include-task / --exclude-task", async (t) => { | ||
| const {argv, serve, server} = t.context; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,6 +149,42 @@ test.serial("subscribe: polling backend is loaded and used when UI5_WATCH_MODE=p | |
| } | ||
| }); | ||
|
|
||
| test.serial("subscribe: returns an inert subscription when UI5_WATCH_MODE=off", async (t) => { | ||
| // Disabled mode must not load any backend. The native stub stands in as a tripwire: if subscribe() | ||
| // delegated, it would be called. The returned subscription is real enough to track and unsubscribe. | ||
| process.env.UI5_WATCH_MODE = "off"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Running this in an non isolated environment will set this to Maybe it will be good if we set this for the sake of the test, but in the end to restore its original value, like we do here, for example: https://github.com/UI5/cli/blob/main/packages/cli/test/lib/dataDir.js#L27-L31 |
||
| const parcelSubscribe = sinon.stub().resolves({unsubscribe: sinon.stub().resolves()}); | ||
| const watcher = await importWatcherWithParcel({ | ||
| default: {subscribe: parcelSubscribe}, subscribe: parcelSubscribe, | ||
| }); | ||
| try { | ||
| const cb = sinon.stub(); | ||
| const subscription = await watcher.subscribe("/some/dir", cb, {ignore: ["**/x/**"]}); | ||
|
|
||
| t.is(parcelSubscribe.callCount, 0, "no native backend is loaded when watching is disabled"); | ||
| t.is(cb.callCount, 0, "the callback is never invoked"); | ||
| t.is(typeof subscription.unsubscribe, "function", "returns a subscription with unsubscribe()"); | ||
| await t.notThrowsAsync(subscription.unsubscribe(), "unsubscribe is a no-op that resolves"); | ||
| } finally { | ||
| esmock.purge(watcher); | ||
| } | ||
| }); | ||
|
|
||
| test.serial("isWatchingDisabled: true only for UI5_WATCH_MODE=off", async (t) => { | ||
| process.env.UI5_WATCH_MODE = "off"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like my previous comment here: https://github.com/UI5/cli/pull/1565/changes#r4105108570 |
||
| let watcher = await importWatcher(); | ||
| t.true(watcher.isWatchingDisabled(), "off disables watching"); | ||
| t.false(watcher.shouldUsePolling(), "off is not polling"); | ||
|
|
||
| process.env.UI5_WATCH_MODE = "polling"; | ||
| watcher = await importWatcher(); | ||
| t.false(watcher.isWatchingDisabled(), "polling does not disable watching"); | ||
|
|
||
| process.env.UI5_WATCH_MODE = "native"; | ||
| watcher = await importWatcher(); | ||
| t.false(watcher.isWatchingDisabled(), "native does not disable watching"); | ||
| }); | ||
|
|
||
| test.serial("shouldUsePolling: UI5_WATCH_MODE forces the backend without inspecting the environment", async (t) => { | ||
| const existsSync = sinon.stub().returns(false); | ||
| process.env.UI5_WATCH_MODE = "polling"; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, in that case it would be good also to ensure/test that
live-reloaddisabling logs an info message.