[GitHub] [beam] KhaninArtur commented on a change in pull request #16172: [BEAM-13417] [Playground] Add java preparator for run katas

[GitBox <gi...@apache.org>]

KhaninArtur commented on a change in pull request #16172:
URL: https://github.com/apache/beam/pull/16172#discussion_r765651081



##########
File path: playground/backend/internal/preparators/java_preparators.go
##########
@@ -48,6 +48,19 @@ func GetJavaPreparators(filePath string) *[]Preparator {
 	return &[]Preparator{publicClassModification, additionalPackage}
 }
 
+// GetJavaPreparatorsForKatas returns preparation methods that should be applied to katas with Java code
+func GetJavaPreparatorsForKatas(filePath string) *[]Preparator {
+	publicClassModification := Preparator{
+		Prepare: replace,
+		Args:    []interface{}{filePath, classWithPublicModifierPattern, classWithoutPublicModifierPattern},
+	}
+	removePackage := Preparator{
+		Prepare: replace,
+		Args:    []interface{}{filePath, packagePattern, ``},

Review comment:
       Let's make the empty string a constant

##########
File path: playground/backend/internal/executors/executor.go
##########
@@ -76,8 +76,12 @@ func (ex *Executor) Validate() func(chan bool, chan error, *sync.Map) {
 }
 
 // Prepare returns the function that applies all preparations of executor
-func (ex *Executor) Prepare() func(chan bool, chan error) {
-	return func(doneCh chan bool, errCh chan error) {
+func (ex *Executor) Prepare() func(chan bool, chan error, *sync.Map, string) {
+	return func(doneCh chan bool, errCh chan error, valRes *sync.Map, filePath string) {
+		isKata, ok := valRes.Load(validators.KatasValidatorName)
+		if ok && isKata.(bool) {
+			ex.preparators = *preparators.GetJavaPreparatorsForKatas(filePath)
+		}

Review comment:
       It doesn't seem a good idea to add here Java-specific logic. What do you think about the creation of the `processValidationResults` function in `code_processing.go` that will change the preparators according to the SDK and validation results?

##########
File path: playground/backend/internal/validators/java_validators_test.go
##########
@@ -56,12 +56,14 @@ func writeFile(path string, code string) {
 	}
 }
 
-func TestCheckIsUnitTests(t *testing.T) {
-	testValidatorArgs := make([]interface{}, 1)
+func TestCheckPipelineType(t *testing.T) {
+	testValidatorArgs := make([]interface{}, 3)
 	testValidatorArgs[0] = unitTestFilePath
+	testValidatorArgs[2] = javaUnitTestPattern

Review comment:
       Let's add unit test for katas

##########
File path: playground/backend/internal/validators/java_validators.go
##########
@@ -39,21 +40,26 @@ func GetJavaValidators(filePath string) *[]Validator {
 		Name:      "Valid path",
 	}
 	unitTestValidator := Validator{
-		Validator: CheckIsUnitTests,
-		Args:      validatorArgs,
+		Validator: CheckPipelineType,
+		Args:      append(validatorArgs, javaUnitTestPattern),
 		Name:      UnitTestValidatorName,
 	}
-	validators := []Validator{pathCheckerValidator, unitTestValidator}
+	katasValidator := Validator{
+		Validator: CheckPipelineType,
+		Args:      append(validatorArgs, javaKatasPattern),
+		Name:      KatasValidatorName,
+	}
+	validators := []Validator{pathCheckerValidator, unitTestValidator, katasValidator}
 	return &validators
 }
 
-func CheckIsUnitTests(args ...interface{}) (bool, error) {
+func CheckPipelineType(args ...interface{}) (bool, error) {
 	filePath := args[0].(string)
 	code, err := ioutil.ReadFile(filePath)
 	if err != nil {
 		logger.Errorf("Validation: Error during open file: %s, err: %s\n", filePath, err.Error())
 		return false, err
 	}
-	// check whether s contains substring unit test
-	return strings.Contains(string(code), javaUnitTestPattern), nil
+	// check whether s contains substring unit test or katas
+	return strings.Contains(string(code), args[2].(string)), nil

Review comment:
       Let's extract this pattern next to the `filePath` argument extraction like
   ```go
   pattern := args[2].(string)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org