-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Avoid excessive filesystem read/writes during test execution #9795
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
Avoid excessive filesystem read/writes during test execution #9795
Conversation
@@ -7,66 +7,42 @@ import java.nio.file.Path | |||
|
|||
object ResultFileOpsJsonSpec extends ZIOBaseSpec { | |||
def spec = suite("ResultFileOpsJsonSpec")( | |||
test("simple write")( | |||
test("writes lines within the result array sequentially")( |
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.
The previous testing didn't have this, but given the special casing for commas, should we have that in the test?
Otherwise LGTM
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.
The previous testing didn't have this
I know but I wanted to have a more "deterministic" test that wrote things sequentially and a non-deterministic one that tested parallel writes
but given the special casing for commas, should we have that in the test
Good point, I'll add this
} | ||
.ignore | ||
) | ||
private[test] class Json private (resultPath: String) extends ResultFileOps { |
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.
🟢 Could be final
?
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.
LGTM
/fixes #9783
Currently we're appending the result of each test to a file. When test execution finishes, we load up the entire file, and then write each line by opening / closing a new
java.io.FileWriter
. This is very inefficient especially for large test suites.With this PR we store all the writes in a queue which we then drain and write its contents to the FS once we close
ResultFileOps
.Previously, running
coreTestsJVM/test
locally required 10 seconds (!!!) for the test reports to be written to the file, whereas now it's done in a matter of milliseconds.