From 2c13d5ebf7e4eac2add96168a99b5ac4113aaa3d Mon Sep 17 00:00:00 2001 From: lhchavez Date: Fri, 17 May 2019 02:00:09 +0000 Subject: [PATCH] Better messaging for custom validators This change adds better messaging when custom validators are involved, which should reduce the amount of guesswork. --- gitservertest/zip.go | 28 +++++ handler.go | 4 +- ziphandler.go | 21 ---- ziphandler_test.go | 252 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 280 insertions(+), 25 deletions(-) diff --git a/gitservertest/zip.go b/gitservertest/zip.go index 2c9b9aa..8e5d410 100644 --- a/gitservertest/zip.go +++ b/gitservertest/zip.go @@ -33,6 +33,34 @@ const ( "Name": "token-caseless" } } +` + + // CustomValidatorSettingsJSON is the JSON representation of a problem with + // just one case called "0" and a custom validator. + CustomValidatorSettingsJSON = `{ + "Cases": [ + { + "Cases": [ + { + "Name": "0", + "Weight": 1 + } + ], + "Name": "0" + } + ], + "Limits": { + "ExtraWallTime": "0s", + "MemoryLimit": 33554432, + "OutputLimit": 10240, + "OverallWallTimeLimit": "1m0s", + "TimeLimit": "1s" + }, + "Slow": false, + "Validator": { + "Name": "custom" + } +} ` ) diff --git a/handler.go b/handler.go index 34006e2..1ceaacd 100644 --- a/handler.go +++ b/handler.go @@ -751,7 +751,9 @@ func validateUpdateMaster( return base.ErrorWithCategory( ErrProblemBadLayout, errors.Errorf( - "problem with unused validator", + "problem requested using validator %s, but has an unused validator.%s file", + problemSettings.Validator.Name, + validatorLang, ), ) } diff --git a/ziphandler.go b/ziphandler.go index 312155a..27bc42c 100644 --- a/ziphandler.go +++ b/ziphandler.go @@ -955,7 +955,6 @@ func CreatePackfile( func getUpdatedProblemSettings( problemSettings *common.ProblemSettings, - zipMergeStrategy ZipMergeStrategy, repo *git.Repository, parent *git.Oid, ) (*common.ProblemSettings, error) { @@ -1015,25 +1014,6 @@ func getUpdatedProblemSettings( ) } - if updatedProblemSettings.Validator.Name != problemSettings.Validator.Name { - if updatedProblemSettings.Validator.Name == "custom" { - return nil, base.ErrorWithCategory( - ErrProblemBadLayout, - errors.Errorf( - "problem with unused validator", - ), - ) - } - if problemSettings.Validator.Name == "custom" { - return nil, base.ErrorWithCategory( - ErrProblemBadLayout, - errors.Errorf( - "problem with custom validator missing a validator", - ), - ) - } - } - updatedProblemSettings.Limits = problemSettings.Limits updatedProblemSettings.Validator.Name = problemSettings.Validator.Name updatedProblemSettings.Validator.Tolerance = problemSettings.Validator.Tolerance @@ -1139,7 +1119,6 @@ func ConvertZipToPackfile( var err error if settings, err = getUpdatedProblemSettings( settings, - zipMergeStrategy, repo, parent, ); err != nil { diff --git a/ziphandler_test.go b/ziphandler_test.go index 8bea40f..c743daa 100644 --- a/ziphandler_test.go +++ b/ziphandler_test.go @@ -111,7 +111,7 @@ func postZip( } func TestPushZip(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "handler_test") + tmpDir, err := ioutil.TempDir("", t.Name()) if err != nil { t.Fatalf("Failed to create directory: %v", err) } @@ -184,7 +184,7 @@ func TestPushZip(t *testing.T) { } func TestConvertZip(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "handler_test") + tmpDir, err := ioutil.TempDir("", t.Name()) if err != nil { t.Fatalf("Failed to create directory: %v", err) } @@ -264,7 +264,7 @@ func TestConvertZip(t *testing.T) { } func TestUpdateProblemSettings(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "handler_test") + tmpDir, err := ioutil.TempDir("", t.Name()) if err != nil { t.Fatalf("Failed to create directory: %v", err) } @@ -397,3 +397,249 @@ func TestUpdateProblemSettings(t *testing.T) { } } } + +func TestUpdateProblemSettingsWithCustomValidator(t *testing.T) { + tmpDir, err := ioutil.TempDir("", t.Name()) + if err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + if os.Getenv("PRESERVE") == "" { + defer os.RemoveAll(tmpDir) + } + + log := base.StderrLog() + ts := httptest.NewServer(ZipHandler( + tmpDir, + NewGitProtocol(authorize, nil, true, OverallWallTimeHardLimit, fakeInteractiveSettingsCompiler, log), + &base.NoOpMetrics{}, + log, + )) + defer ts.Close() + + problemAlias := "sumas-validator" + + // Create the problem. + { + zipContents, err := gitservertest.CreateZip( + map[string]io.Reader{ + "settings.json": strings.NewReader(gitservertest.CustomValidatorSettingsJSON), + "cases/0.in": strings.NewReader("1 2\n"), + "cases/0.out": strings.NewReader("3\n"), + "statements/es.markdown": strings.NewReader("Sumaz\n"), + "validator.py": strings.NewReader("print 1\n"), + }, + ) + if err != nil { + t.Fatalf("Failed to create zip: %v", err) + } + postZip( + t, + adminAuthorization, + problemAlias, + nil, + ZipMergeStrategyTheirs, + zipContents, + "initial commit", + true, // create + true, // useMultipartFormData + ts, + ) + } + + // Update settings. + { + zipContents, err := gitservertest.CreateZip( + map[string]io.Reader{}, + ) + if err != nil { + t.Fatalf("Failed to create zip: %v", err) + } + problemSettings := &common.ProblemSettings{ + Limits: common.LimitsSettings{ + ExtraWallTime: base.Duration(0), + MemoryLimit: 33554432, + OutputLimit: 10240, + OverallWallTimeLimit: base.Duration(5 * time.Minute), + TimeLimit: base.Duration(time.Second), + }, + Slow: false, + Validator: common.ValidatorSettings{ + Name: "custom", + }, + } + updateResult := postZip( + t, + adminAuthorization, + problemAlias, + problemSettings, + ZipMergeStrategyOurs, + zipContents, + "updated settings", + false, // create + false, // useMultipartFormData + ts, + ) + + updatedFiles := make(map[string]struct{}) + for _, updatedFile := range updateResult.UpdatedFiles { + updatedFiles[updatedFile.Path] = struct{}{} + if updatedFile.Type != "modified" { + t.Errorf("unexpected updated file: %v", updatedFile) + } + } + + expectedUpdatedFiles := map[string]struct{}{ + "settings.json": {}, + "settings.distrib.json": {}, + } + if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) { + t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles) + } + } + + // Update statements. + { + zipContents, err := gitservertest.CreateZip( + map[string]io.Reader{ + "statements/es.markdown": strings.NewReader("Sumas\n"), + }, + ) + if err != nil { + t.Fatalf("Failed to create zip: %v", err) + } + updateResult := postZip( + t, + adminAuthorization, + problemAlias, + nil, + ZipMergeStrategyRecursiveTheirs, + zipContents, + "updated statement", + false, // create + false, // useMultipartFormData + ts, + ) + + updatedFiles := make(map[string]struct{}) + for _, updatedFile := range updateResult.UpdatedFiles { + updatedFiles[updatedFile.Path] = struct{}{} + if updatedFile.Type != "modified" { + t.Errorf("unexpected updated file: %v", updatedFile) + } + } + + expectedUpdatedFiles := map[string]struct{}{ + "statements/es.markdown": {}, + } + if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) { + t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles) + } + } + + // Use token validator. + { + zipContents, err := gitservertest.CreateZip( + map[string]io.Reader{ + "cases/0.in": strings.NewReader("1 2\n"), + "cases/0.out": strings.NewReader("3\n"), + "statements/es.markdown": strings.NewReader("Sumas\n"), + }, + ) + if err != nil { + t.Fatalf("Failed to create zip: %v", err) + } + problemSettings := &common.ProblemSettings{ + Limits: common.LimitsSettings{ + ExtraWallTime: base.Duration(0), + MemoryLimit: 33554432, + OutputLimit: 10240, + OverallWallTimeLimit: base.Duration(5 * time.Minute), + TimeLimit: base.Duration(time.Second), + }, + Slow: false, + Validator: common.ValidatorSettings{ + Name: "token-caseless", + }, + } + updateResult := postZip( + t, + adminAuthorization, + problemAlias, + problemSettings, + ZipMergeStrategyTheirs, + zipContents, + "updated validator", + false, // create + false, // useMultipartFormData + ts, + ) + + updatedFiles := make(map[string]string) + for _, updatedFile := range updateResult.UpdatedFiles { + updatedFiles[updatedFile.Path] = updatedFile.Type + } + + expectedUpdatedFiles := map[string]string{ + "settings.distrib.json": "modified", + "settings.json": "modified", + "validator.py": "deleted", + } + if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) { + t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles) + } + } + + // Restore custom validator. + { + zipContents, err := gitservertest.CreateZip( + map[string]io.Reader{ + "cases/0.in": strings.NewReader("1 2\n"), + "cases/0.out": strings.NewReader("3\n"), + "statements/es.markdown": strings.NewReader("Sumas\n"), + "validator.py": strings.NewReader("print 1\n"), + }, + ) + if err != nil { + t.Fatalf("Failed to create zip: %v", err) + } + problemSettings := &common.ProblemSettings{ + Limits: common.LimitsSettings{ + ExtraWallTime: base.Duration(0), + MemoryLimit: 33554432, + OutputLimit: 10240, + OverallWallTimeLimit: base.Duration(5 * time.Minute), + TimeLimit: base.Duration(time.Second), + }, + Slow: false, + Validator: common.ValidatorSettings{ + Name: "custom", + }, + } + updateResult := postZip( + t, + adminAuthorization, + problemAlias, + problemSettings, + ZipMergeStrategyTheirs, + zipContents, + "updated validator", + false, // create + false, // useMultipartFormData + ts, + ) + + updatedFiles := make(map[string]string) + for _, updatedFile := range updateResult.UpdatedFiles { + updatedFiles[updatedFile.Path] = updatedFile.Type + } + + expectedUpdatedFiles := map[string]string{ + "settings.distrib.json": "modified", + "settings.json": "modified", + "validator.py": "added", + } + if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) { + t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles) + } + } +}