Skip to content

Commit

Permalink
Address Feedback From PR Review
Browse files Browse the repository at this point in the history
Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
mahadzaryab1 committed Sep 2, 2024
1 parent c69cbb6 commit ed259ec
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 11 deletions.
4 changes: 0 additions & 4 deletions cmd/query/app/static_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ type loadedConfig struct {

// NewStaticAssetsHandler returns a StaticAssetsHandler
func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandlerOptions) (*StaticAssetsHandler, error) {
if options.Logger == nil {
options.Logger = zap.NewNop()
}

assetsFS := ui.GetStaticFiles(options.Logger)
if staticAssetsRoot != "" {
assetsFS = http.Dir(staticAssetsRoot)
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/ui/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var placeholderAssetsFS embed.FS
// non-functional index.html is returned to be used as a placeholder.
func GetStaticFiles(logger *zap.Logger) http.FileSystem {
if _, err := actualAssetsFS.ReadFile("actual/index.html.gz"); err != nil {
logger.Warn("ui assets not found", zap.Error(err))
logger.Warn("ui assets not embedded in the binary, using a placeholder", zap.Error(err))
return httpfs.PrefixedFS("placeholder", http.FS(placeholderAssetsFS))
}

Expand Down
13 changes: 7 additions & 6 deletions cmd/query/app/ui/assets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ package ui
import (
"embed"
"io"
"strings"
"testing"

"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/testutils"
)

func TestGetStaticFiles_ReturnsPlaceholderWhenActualNotPresent(t *testing.T) {
Expand All @@ -20,11 +20,12 @@ func TestGetStaticFiles_ReturnsPlaceholderWhenActualNotPresent(t *testing.T) {
actualAssetsFS = embed.FS{}
t.Cleanup(func() { actualAssetsFS = currentActualAssets })

fs := GetStaticFiles(zap.NewNop())
logger, logBuf := testutils.NewLogger()
fs := GetStaticFiles(logger)
file, err := fs.Open("/index.html")
require.NoError(t, err)
buf := new(strings.Builder)
_, err = io.Copy(buf, file)
bytes, err := io.ReadAll(file)
require.NoError(t, err)
require.Contains(t, buf.String(), "This is a placeholder for the Jaeger UI home page")
require.Contains(t, string(bytes), "This is a placeholder for the Jaeger UI home page")
require.Contains(t, logBuf.String(), "ui assets not embedded in the binary, using a placeholder")
}

0 comments on commit ed259ec

Please sign in to comment.