-
Notifications
You must be signed in to change notification settings - Fork 885
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
Consolidate CI test output into a single string #8489
base: main
Are you sure you want to change the base?
Conversation
Having CI test process stdout and stderr in a single string makes it easier to read when looking through failures, since you can see the test output alongside the error messages. Without this, any stderr output written during a test will be captured seperately from the test output, so when we then log it after a test failure, we can't tell which test logged which errors.
|
Size Report 1Affected ProductsNo changes between base commit (3670ab8) and merge commit (7b91ab1).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (3670ab8) and merge commit (7b91ab1).Test Logs |
I'm going to defer to @hsubox76 on this one. Perhaps there was a reason to keep them separated originally? |
@@ -74,27 +73,26 @@ const argv = yargs.options({ | |||
const testProcess = spawn('yarn', ['--cwd', dir, scriptName]); | |||
|
|||
testProcess.childProcess.stdout.on('data', data => { | |||
stdout += data.toString(); | |||
testProcessOutput += data.toString(); |
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'm not sure if it is ever useful to know which output came out of stdout and which came out of stderr but just in case it might be, do you think we might want to append a prefix like [stdout]
or [stderr]
before each of these?
Having the
stdout
andstderr
in a single string makes it easier to read when analyzing test failures. Currently if there's a test failure, we output stdout and stderr separately so we can't tell which errors were logged during which test.For example, in https://github.com/firebase/firebase-js-sdk/actions/runs/10776099172/job/29882046607, there are tons of
FIREBASE FATAL ERROR
's logged, but I can't tell which tests are logging these errors, or if they're even what's causing the tests to fail.