-
-
Notifications
You must be signed in to change notification settings - Fork 650
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
JUnit: create directory before creating output file #555
base: master
Are you sure you want to change the base?
Conversation
reporters/junit_reporter_test.go
Outdated
|
||
When("output directory doesn't exist", func() { | ||
It("should create before open file", func() { | ||
output := "/tmp/not/exists/report.xml" |
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.
Can we use ioutil.TempDir()
here to create an empty directory and then reference a non-existing-file.xml
. This allows the tests to be isolated, and also support systems that don't have a /
directory structure.
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.
There’s actually two assumptions being made here:
- as Will pointed at
/...
assumes the directory structure /tmp
assumes the “correct” location of temporary files
ioutil.TempDir
actually looks at os.TempDir
to determine where temporary files should live - which can vary across Windows, OSX, Linux, etc. So using ioutil.TempDir
is more likely to be correct.
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.
Thanks for your review. I fixed it by use ioutil.TempDir
to create temp directory and use filepath.Join
to join the outputfile to make it work with Windows.
@@ -124,6 +125,9 @@ func (reporter *JUnitReporter) SpecSuiteDidEnd(summary *types.SuiteSummary) { | |||
reporter.suite.Time = math.Trunc(summary.RunTime.Seconds()*1000) / 1000 | |||
reporter.suite.Failures = summary.NumberOfFailedSpecs | |||
reporter.suite.Errors = 0 | |||
if err := os.MkdirAll(filepath.Dir(reporter.filename), 0755); err != nil { | |||
fmt.Printf("Failed to create JUnit report file: %s\n\t%s", reporter.filename, err.Error()) |
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.
It would be nice if we could test this error, maybe by providing a super long path name perhaps. May be hard to drive out but would be good.
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.
It would be hard to test because SpecSuiteDidEnd
doesn't return an error and we used fmt.Printf
as a returned message.
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.
I think in this case we should return immediately after print a message and assert report file should not exist. What do you think?
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.
I just try to write a test case that create with too long folder:
FIt("should print an error if directory cannot be create", func() {
dir, err := ioutil.TempDir("", "not-exist")
Expect(err).ShouldNot(HaveOccurred())
// make directory name over os limit.
var name string
for i := 0; i < 256; i++ {
name += "a"
}
output := filepath.Join(dir, name, "report.xml")
reporter := reporters.NewJUnitReporter(output)
reporter.SpecSuiteDidEnd(&types.SuiteSummary{
NumberOfSpecsThatWillBeRun: 1,
NumberOfFailedSpecs: 0,
RunTime: testSuiteTime,
})
Ω(output).ShouldNot(BeAnExistingFile())
})
As I understand correctly, the test case above should pass but it failed:
Running Suite: Reporters Suite
==============================
Random Seed: 1546490671
Will run 1 of 48 specs
SSSSSSSSSSSSSSSSSFailed to create JUnit report file: /var/folders/x7/tl6gyzn92vzcr35t94xgt6y00000gp/T/not-exist738463321/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/report.xml
mkdir /var/folders/x7/tl6gyzn92vzcr35t94xgt6y00000gp/T/not-exist738463321/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: file name too longFailed to create JUnit report file: /var/folders/x7/tl6gyzn92vzcr35t94xgt6y00000gp/T/not-exist738463321/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/report.xml
open /var/folders/x7/tl6gyzn92vzcr35t94xgt6y00000gp/T/not-exist738463321/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/report.xml: file name too longFailed to generate JUnit report
invalid argument
------------------------------
• Failure [0.001 seconds]
JUnit Reporter
/Users/thanabodee/src/github.com/onsi/ginkgo/reporters/junit_reporter_test.go:18
when output directory doesn't exist
/Users/thanabodee/src/github.com/onsi/ginkgo/reporters/junit_reporter_test.go:260
should print an error if directory cannot be create [It]
/Users/thanabodee/src/github.com/onsi/ginkgo/reporters/junit_reporter_test.go:276
stat /var/folders/x7/tl6gyzn92vzcr35t94xgt6y00000gp/T/not-exist738463321/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/report.xml: file name too long
/Users/thanabodee/src/github.com/onsi/ginkgo/reporters/junit_reporter_test.go:292
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
Summarizing 1 Failure:
[Fail] JUnit Reporter when output directory doesn't exist [It] should print an error if directory cannot be create
/Users/thanabodee/src/github.com/onsi/ginkgo/reporters/junit_reporter_test.go:292
Ran 1 of 48 Specs in 0.001 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 47 Skipped
--- FAIL: TestReporters (0.00s)
FAIL
Ginkgo ran 1 suite in 985.136553ms
Test Suite Failed
That's happen because Gomega doesn't not check file path is too long at this line. Is it a bug? Should be report to gonega repository?
@williammartin @BooleanCat first comment got fixed. but the second comment I'm stuck with gomega . I already add the detail in second comment. Please take a look. Thanks |
Close #554