Skip to content

[BEAM-13623][Playground] [Bugfix] During unit tests failing there is no any output #16469

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.

Sign up for GitHub

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

Merged
pabloem merged 2 commits into from
Jan 12, 2022
+15 −11
Merged

[BEAM-13623][Playground] [Bugfix] During unit tests failing there is no any output #16469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3731b9
[BEAM-13623][Playground]
Jan 10, 2022
ea54381
[BEAM-13623][Playground]
Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions playground/backend/internal/code_processing/code_processing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,13 @@ func Process(ctx context.Context, cacheService cache.Cache, lc *fs_tool.LifeCycl
return
}
if !ok {
if runOutput.Error != nil {
runError.Write([]byte(runOutput.Error.Error()))
// If unit test has some error then error output is placed as RunOutput
if isUnitTest {
Copy link
Contributor

@KhaninArtur KhaninArtur Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add an else case to do this?

if runOutput.Error != nil {
	runError.Write([]byte(runOutput.Error.Error()))

Copy link
Contributor Author

@AydarZaynutdinov AydarZaynutdinov Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, no. I removed runOutput.Error field from run_output_writer.go. We do not need this field. This error (runOutput.Error.Error()) exists in errorChannel and be thrown to the log without using runError.Write([]byte(runOutput.Error.Error())). So we do not need this one:

if runOutput.Error != nil {
	runError.Write([]byte(runOutput.Error.Error()))

This PR is to receive an error's output that occurs during the processing of the unit tests. This error output is placed in the cache as a RunOutput so we need to get this one and keep it to the runError value but only if it is a unit tests (isUnitTest) and there is an error during processing (!ok)

output, err := GetProcessingOutput(ctx, cacheService, pipelineId, cache.RunOutput, "")
if err == nil {
runError.Write([]byte(output))
}
Comment on lines +195 to +197
Copy link
Contributor

@KhaninArtur KhaninArtur Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to process this err if it is not nil?

Copy link
Contributor Author

@AydarZaynutdinov AydarZaynutdinov Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error occurs only if GetProcessingOutput method couldn't receive the correct run output (there is no run output in the cache or it is no string value). In this case, GetProcessingOutput method logs this error.

Receiving this error we do not need to send it to the other methods or to log it, so we just will not use this error, because on the next lines we call processRunError method after which the whole code processing will be finished.


}
// Run step is finished, but code contains some error (divide by 0 for example)
if sdkEnv.ApacheBeamSdk == pb.Sdk_SDK_GO {
Expand Down Expand Up @@ -431,7 +436,7 @@ func processError(ctx context.Context, errorChannel chan error, pipelineId uuid.
func processErrorWithSavingOutput(ctx context.Context, err error, errorOutput []byte, pipelineId uuid.UUID, subKey cache.SubKey, cacheService cache.Cache, errorTitle string, newStatus pb.Status) error {
logger.Errorf("%s: %s(): err: %s, output: %s\n", pipelineId, errorTitle, err.Error(), errorOutput)

if err := utils.SetToCache(ctx, cacheService, pipelineId, subKey, fmt.Sprintf("error: %s, output: %s", err.Error(), errorOutput)); err != nil {
if err := utils.SetToCache(ctx, cacheService, pipelineId, subKey, fmt.Sprintf("error: %s\noutput: %s", err.Error(), errorOutput)); err != nil {
return err
}

Expand All @@ -446,7 +451,7 @@ func processRunError(ctx context.Context, errorChannel chan error, errorOutput [
err := <-errorChannel
logger.Errorf("%s: Run(): err: %s, output: %s\n", pipelineId, err.Error(), errorOutput)

if err := utils.SetToCache(ctx, cacheService, pipelineId, cache.RunError, fmt.Sprintf("error: %s, output: %s", err.Error(), string(errorOutput))); err != nil {
if err := utils.SetToCache(ctx, cacheService, pipelineId, cache.RunError, fmt.Sprintf("error: %s\noutput: %s", err.Error(), string(errorOutput))); err != nil {
return err
}

Expand Down
4 changes: 2 additions & 2 deletions playground/backend/internal/code_processing/code_processing_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func Test_Process(t *testing.T) {
code: "MOCK_CODE",
cancelFunc: false,
expectedStatus: pb.Status_STATUS_COMPILE_ERROR,
expectedCompileOutput: "error: exit status 1, output: %s:1: error: reached end of file while parsing\nMOCK_CODE\n^\n1 error\n",
expectedCompileOutput: "error: exit status 1\noutput: %s:1: error: reached end of file while parsing\nMOCK_CODE\n^\n1 error\n",
expectedRunOutput: nil,
expectedRunError: nil,
args: args{
Expand All @@ -193,7 +193,7 @@ func Test_Process(t *testing.T) {
expectedStatus: pb.Status_STATUS_RUN_ERROR,
expectedCompileOutput: "",
expectedRunOutput: "",
expectedRunError: "error: exit status 1, output: Exception in thread \"main\" java.lang.ArithmeticException: / by zero\n\tat HelloWorld.main(%s.java:3)\n",
expectedRunError: "error: exit status 1\noutput: Exception in thread \"main\" java.lang.ArithmeticException: / by zero\n\tat HelloWorld.main(%s.java:3)\n",
args: args{
ctx: context.Background(),
appEnv: appEnvs,
Expand Down
9 changes: 4 additions & 5 deletions playground/backend/internal/streaming/run_output_writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ type RunOutputWriter struct {
Ctx context.Context
CacheService cache.Cache
PipelineId uuid.UUID
Error error
}

// Write writes len(p) bytes from p to cache with cache.RunOutput subKey.
Expand All @@ -46,8 +45,8 @@ func (row *RunOutputWriter) Write(p []byte) (int, error) {

prevOutput, err := row.CacheService.GetValue(row.Ctx, row.PipelineId, cache.RunOutput)
if err != nil {
row.Error = err
return 0, err
customErr := fmt.Errorf("error during saving output: %s", err)
return 0, customErr
}

// concat prevValue and new value
Expand All @@ -56,8 +55,8 @@ func (row *RunOutputWriter) Write(p []byte) (int, error) {
// set new cache value
err = row.CacheService.SetValue(row.Ctx, row.PipelineId, cache.RunOutput, str)
if err != nil {
row.Error = err
return 0, err
customErr := fmt.Errorf("error during saving output: %s", err)
return 0, customErr
}
return len(p), nil
}