Fix academy test launcher#194
Conversation
Signed-off-by: Raunak Madan <madanraunak24@gmail.com>
|
There was a problem hiding this comment.
Code Review
This pull request refactors the calculation of curriculaRoot in layouts/test/single.html and updates several script variables to use safeJS. A review comment suggests using a regular expression instead of strings.TrimSuffix to more reliably derive the curriculaRoot path.
|
|
||
| {{ $test := partial "test/single.html" . }} | ||
| {{ $tesAbsPath := .RelPermalink }} | ||
| {{ $curriculaRoot := .RelPermalink | strings.TrimSuffix "/" | strings.TrimSuffix "/test" }} |
There was a problem hiding this comment.
The current implementation to derive curriculaRoot using strings.TrimSuffix is a bit fragile. It could lead to incorrect paths if a directory name happens to end with "test" but is not exactly "test" (e.g., /some/path/latest-test/).
A more robust approach would be to use a regular expression to ensure you're removing the /test segment only when it appears at the very end of the path.
Consider this alternative:
{{ $curriculaRoot := .RelPermalink | replaceRE "/test/?$" "" }}This uses replaceRE to remove /test or /test/ from the end of the .RelPermalink, which is safer and more explicit about the intent.
| {{ $curriculaRoot := .RelPermalink | strings.TrimSuffix "/" | strings.TrimSuffix "/test" }} | |
| {{ $curriculaRoot := .RelPermalink | replaceRE "/test/?$" "" }} |
Notes for Reviewers
layouts/test/single.html. The emitted script was producing invalid JS string values.Examples:
before, the launcher could emit
build='"production"'instead ofbuild="production", sobuild === "production"evaluated false and the redirect never ranmeshery-extensibility/test/test?...
The patch uses jsonify | safeJS for launcher literals and trims the trailing /test when deriving the parent redirect base.
Signed commits