feat: check screenshot image type #278
Conversation
s4kh
requested review from
oshirohugo,
wbrowne,
xnyo,
academo,
a team and
andresmgot
and removed request for
a team and
xnyo
| if strings.TrimSpace(screenshot.Path) == "" { | ||
| reportCount++ | ||
| pass.ReportResult(pass.AnalyzerName, screenshots, fmt.Sprintf("plugin.json: invalid empty screenshot path: %q", screenshot.Name), "The screenshot path must not be empty.") | ||
| } else if !validImageType(screenshot.Path) { |
There was a problem hiding this comment.
Is it better to do this after invalid empty screenshot in a separate loop? Added here so we can use the same loop.
s4kh
force-pushed
the
feat-validate-screenshot-ext
branch
2 times, most recently
from
accf0c3 to
d629f7d
Compare
s4kh
force-pushed
the
feat-validate-screenshot-ext
branch
from
d629f7d to
2cb7977
Compare
| pass.ReportResult(pass.AnalyzerName, screenshots, fmt.Sprintf("plugin.json: invalid empty screenshot path: %q", screenshot.Name), "The screenshot path must not be empty.") | ||
| } else if !validImageType(filepath.Join(archiveDir, screenshot.Path)) { | ||
| reportCount++ | ||
| pass.ReportResult(pass.AnalyzerName, screenshotsType, fmt.Sprintf("invalid screenshot image type: %q. Accepted image types: %q", screenshot.Path, acceptedImageTypes), "The screenshot image type invalid.") |
There was a problem hiding this comment.
This error can be confusing for users if for example they have a typo in their plugin json or the wrong path. We'll be telling them the image type is wrong but in reality the image path is wrong.
Let's have an error if the image path can't be found and a separate one for the image type.
| func validImageType(imgPath string) bool { | ||
| file, err := os.Open(imgPath) | ||
| if err != nil { | ||
| fmt.Printf("cannot open file: %v\n", err) |
There was a problem hiding this comment.
don't output to stdout it'll break the validator output. use logme.DebugFln
| // https://pkg.go.dev/net/http#DetectContentType | ||
| buffer := make([]byte, 512) | ||
| if _, err := file.Read(buffer); err != nil { | ||
| fmt.Printf("cannot read file: %v\n", err) |
There was a problem hiding this comment.
don't output to stdout use our logging lib
| @@ -62,18 +65,19 @@ func TestNoScreenshots(t *testing.T) { | |||
| func TestInvalidScreenshotPath(t *testing.T) { | |||
There was a problem hiding this comment.
add a test case for a path that doesn't exist.
With the table test, I was able to catch an SVG validation error, thanks @academo. 💯 |
s4kh
force-pushed
the
feat-validate-screenshot-ext
branch
from
90fadc5 to
d90bdd0
Compare
Fixes #277