You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "gyowoo1113 (via GitHub)" <gi...@apache.org> on 2026/07/17 13:50:21 UTC

[PR] [ZEPPELIN-6435] Remove frontend build dependency from IndexHtmlServletTest [zeppelin]

gyowoo1113 opened a new pull request, #5314:
URL: https://github.com/apache/zeppelin/pull/5314

   ### What is this PR for?
   `IndexHtmlServletTest` included a disabled Angular HTML addon test because it depended on the generated file at `zeppelin-web-angular/dist/zeppelin/index.html`. That artifact is not built during normal `zeppelin-server` tests, so the test could not run independently.
   
   This PR replaces the frontend build dependency with a temporary, test-owned `index.html` file and re-enables `testZeppelinWebAngularHtmlAddon`.
   
   The `zeppelin-web` index and the Angular-style index exercise different branches in `IndexHtmlServlet`. The `zeppelin-web` index does not contain explicit `</head>` and `</body>` closing tags, so the servlet falls back to inserting the head addon before `<body>` and the body addon before `</html>`. The Angular-style index contains both closing tags, so the re-enabled test verifies that the configured head and body addons are inserted immediately before `</head>` and `</body>`.
   
   ### What type of PR is it?
   Improvement
   
   ### Todos
   - [x] Remove the dependency on `zeppelin-web-angular/dist/zeppelin/index.html`
   - [x] Create a temporary, test-owned Angular-style `index.html`
   - [x] Re-enable `testZeppelinWebAngularHtmlAddon`
   - [x] Verify addon insertion before `</head>` and `</body>`
   
   ### What is the Jira issue?
   [[ZEPPELIN-6435]](https://issues.apache.org/jira/browse/ZEPPELIN-6435)
   
   ### How should this be tested?
   `./mvnw test -pl zeppelin-server -Dtest=IndexHtmlServletTest` passes successfully.
   
   ### Screenshots (if appropriate)
   N/A
   
   ### Questions:
   * Does the license files need to update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-6435] Remove frontend build dependency from IndexHtmlServletTest [zeppelin]

Posted by "gyowoo1113 (via GitHub)" <gi...@apache.org>.
gyowoo1113 commented on PR #5314:
URL: https://github.com/apache/zeppelin/pull/5314#issuecomment-5014944502

   Thanks for catching this. I updated `testZeppelinWebHtmlAddon` to use `@TempDir` as well and to verify the fallback behavior using an HTML document without closing tags.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-6435] Remove frontend build dependency from IndexHtmlServletTest [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on PR #5314:
URL: https://github.com/apache/zeppelin/pull/5314#issuecomment-5012132052

   Since the PR's goal is to make `IndexHtmlServletTest` independent of the frontend build, could we apply the same treatment to `testZeppelinWebHtmlAddon` in this PR? It still hardcodes `../zeppelin-web/dist/index.html`, another git-ignored build artifact. It only passes in CI because the core job builds `zeppelin-web` via `-Pweb-classic`; a clean local `./mvnw test -pl zeppelin-server` would fail it for the exact reason you're fixing on the Angular side. Reusing the `@TempDir` pattern here (with a document that omits the closing tags, so it exercises the fallback branch in `IndexHtmlServlet`) would make both tests fully self-contained and complete the stated goal.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-6435] Remove frontend build dependency from IndexHtmlServletTest [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee merged PR #5314:
URL: https://github.com/apache/zeppelin/pull/5314


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-6435] Remove frontend build dependency from IndexHtmlServletTest [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on code in PR #5314:
URL: https://github.com/apache/zeppelin/pull/5314#discussion_r3608781848


##########
zeppelin-server/src/test/java/org/apache/zeppelin/server/IndexHtmlServletTest.java:
##########
@@ -43,8 +44,9 @@ class IndexHtmlServletTest {
     private final static String TEST_HEAD_ADDON = "<!-- bar -->";
 
     private final static String FILE_PATH_INDEX_HTML_ZEPPELIN_WEB = "../zeppelin-web/dist/index.html";
-    private final static String FILE_PATH_INDEX_HTML_ZEPPELIN_WEB_ANGULAR = "../zeppelin-web-angular/dist/zeppelin/index.html";
-
+    

Review Comment:
   ```suggestion
   
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org