Skip to content
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

add workflow e2e test #1318

Merged

Conversation

Yancey1989
Copy link
Collaborator

@Yancey1989 Yancey1989 commented Dec 3, 2019

the part workflow of #1066
This PR adds an e2e test for the SQLFLow workflow.

The workflow step failed on REPL executing the SQL program #1317

cmd/repl/repl.go Outdated
if !isTerminal {
fmt.Println("sqlflow>", stmt)
}
tableRendered := false
table := tablewriter.NewWriter(os.Stdout)

stream := sql.RunSQLProgram(stmt, db, modelDir, &pb.Session{})
stream := sql.RunSQLProgram(stmt, modelDir, &pb.Session{DbConnStr: ds})
Copy link
Collaborator

Choose a reason for hiding this comment

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

cmd/sqlflowserver/main_test.go Show resolved Hide resolved
pkg/sql/codegen/couler/codegen.go Outdated Show resolved Hide resolved
pkg/sql/database.go Show resolved Hide resolved
pkg/sql/executor_ir.go Outdated Show resolved Hide resolved
@Yancey1989 Yancey1989 changed the title add workflow e2e test [WIP]add workflow e2e test Dec 3, 2019
@Yancey1989
Copy link
Collaborator Author

Thanks for @typhoonzero 's review, I found some bug that blocks this e2e test, so I renamed this PR to WIP status and will create some PRs to fix these bugs while updating this e2e test.

This was referenced Dec 3, 2019
@typhoonzero
Copy link
Collaborator

Thanks for @typhoonzero 's review, I found some bug that blocks this e2e test, so I renamed this PR to WIP status and will create some PRs to fix these bugs while updating this e2e test.

Cool!

@@ -478,6 +480,80 @@ func TestEnd2EndMaxComputeElasticDL(t *testing.T) {
t.Run("CaseTrainElasticDL", CaseTrainElasticDL)
}

func TestEnd2EndMySQLArgoMode(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Testing MySQL requires starting a MySQL server inside a Pod. Maybe we should use MaxCompute as data source instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

using MySQL can check each PR that comes from forked PR, maybe it can cover more test case.

typhoonzero
typhoonzero previously approved these changes Dec 6, 2019
@typhoonzero typhoonzero dismissed their stale review December 6, 2019 01:54

Should clean up travis config

@Yancey1989 Yancey1989 changed the title [WIP]add workflow e2e test add workflow e2e test Dec 6, 2019
.travis.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@tonyyang-svail tonyyang-svail left a comment

Choose a reason for hiding this comment

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

LGTM.

@tonyyang-svail tonyyang-svail merged commit 0ac0f3e into sql-machine-learning:develop Dec 6, 2019
@Yancey1989 Yancey1989 deleted the workflow_e2e_test branch December 6, 2019 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants