You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/08/03 19:22:39 UTC
[GitHub] [incubator-devlake] keon94 opened a new pull request, #2677: feat: e2e testing for runner.RunTask
keon94 opened a new pull request, #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677
### ⚠️ Pre Checklist
> Please complete _ALL_ items in this checklist, and remove before submitting
- [ ] I have read through the [Contributing](https://devlake.apache.org/community/) Documentation & [PR Template](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
- [ ] This PR is using a `label` (bug, feature etc.)
- [ ] My code is has necessary documentation (if appropriate)
- [ ] I have added any relevant tests
- [ ] This section (**⚠️ Pre Checklist**) will be removed when submitting PR
# Summary
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes.
Please fill out as many sections below as possible.
-->
### Does this close any open issues?
#2676
### Screenshots
Include any relevant screenshots here.
### Other Information
Any other information that is important to this PR.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972577672
##########
plugins/e2e/model/plugin.go:
##########
@@ -0,0 +1,27 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package model
+
+import "github.com/apache/incubator-devlake/plugins/core"
+
+// do not remove (used by mockery)
+// nolint: unused
+type TestPlugin interface {
+ core.PluginTask
+ core.PluginMeta
+}
Review Comment:
has to be in a non-test file to be recognized by mockery
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972559445
##########
plugins/core/e2e/plugin.go:
##########
@@ -0,0 +1,56 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/plugins/core"
+)
+
+type TestPlugin struct {
Review Comment:
It's just a helper for the tests + no tests in it. It won't be compiled against any actual source file
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
klesh commented on PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#issuecomment-1280699479
I would prefer writing unit-test for `runner` instead of e2e-test.
The problems we ran into in the past were things likes "progress blocking the whole system", and "failed to handle panic". Check the https://github.com/apache/incubator-devlake/commits/main/runner for detail.
It would make more sense for us to cook a set of unit-tests to cover those critical issues we met in the past.
These e2e-tests involve too many modules and provide limited guarantees in contrast to the maintenance overhead..
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972561079
##########
runner/run_task.go:
##########
@@ -43,10 +43,10 @@ func RunTask(
db *gorm.DB,
progress chan core.RunningProgress,
taskId uint64,
-) errors.Error {
+) (err errors.Error) {
Review Comment:
this was a bug the tests caught
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972574058
##########
plugins/e2e/plugin_runner_test.go:
##########
@@ -0,0 +1,490 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "context"
+ "encoding/json"
+ "github.com/apache/incubator-devlake/config"
+ "github.com/apache/incubator-devlake/errors"
+ "github.com/apache/incubator-devlake/helpers/e2ehelper"
+ "github.com/apache/incubator-devlake/migration"
+ "github.com/apache/incubator-devlake/mocks"
+ "github.com/apache/incubator-devlake/models"
+ "github.com/apache/incubator-devlake/models/migrationscripts"
+ "github.com/apache/incubator-devlake/plugins/core"
+ gitlabhack "github.com/apache/incubator-devlake/plugins/gitlab/api"
+ "github.com/apache/incubator-devlake/runner"
+ "github.com/stretchr/testify/require"
+ "gorm.io/datatypes"
+ "sync"
+ "testing"
+)
+
+var once = &sync.Once{}
+
+func TestPluginRunner(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ taskCtx.GetLogger().Info("running task in %s", pluginHelper.mock.RootPkgPath())
+ connectionId, ok := options["ConnectionId"]
+ require.True(t, ok)
+ require.Equal(t, 1.0, connectionId)
+ return nil, nil
+ }).Once()
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.NoError(t, err)
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 1,
+ FinishedSubTasks: 1,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask1",
+ SubTaskNumber: 1,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_COMPLETED, response.result.Status)
+}
+
+func TestPluginRunner_AutoRunRequiredTask(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ taskCtx.GetLogger().Info("running task in %s", pluginHelper.mock.RootPkgPath())
+ connectionId, ok := options["ConnectionId"]
+ require.True(t, ok)
+ require.Equal(t, 1.0, connectionId)
+ return nil, nil
+ }).Once()
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ return nil
+ },
+ Required: true,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.NoError(t, err)
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 1,
+ FinishedSubTasks: 1,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask1",
+ SubTaskNumber: 1,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_COMPLETED, response.result.Status)
+}
+
+func TestPluginRunner_PrepareError(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ taskCtx.GetLogger().Info("running task in %s", pluginHelper.mock.RootPkgPath())
+ return nil, errors.Default.New("prepare task error")
+ }).Once()
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.Error(t, err)
+ require.Contains(t, err.Messages().Format(), "prepare task error")
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 0,
+ FinishedSubTasks: 0,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "",
+ SubTaskNumber: 0,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_FAILED, response.result.Status)
+}
+
+func TestPluginRunner_EntrypointError(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ return nil, nil
+ }).Once()
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ return errors.Default.New("entrypoint error")
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.Error(t, err)
+ require.Contains(t, err.Messages().Format(), "entrypoint error")
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 1,
+ FinishedSubTasks: 0,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask1",
+ SubTaskNumber: 1,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_FAILED, response.result.Status)
+}
+
+func TestPluginRunner_WithPrepareData(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ return "test data", nil
+ }).Once()
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ require.Equal(t, "test data", c.GetData())
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.NoError(t, err)
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 1,
+ FinishedSubTasks: 1,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask1",
+ SubTaskNumber: 1,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_COMPLETED, response.result.Status)
+}
+
+func TestPluginRunner_twoSubtasks(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ taskCtx.GetLogger().Info("running task in %s", pluginHelper.mock.RootPkgPath())
+ connectionId, ok := options["ConnectionId"]
+ require.True(t, ok)
+ require.Equal(t, 1.0, connectionId)
+ return nil, nil
+ }).Once()
+ var subtaskResponses []string
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ subtaskResponses = append(subtaskResponses, "subtask1")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ {
+ Name: "subtask2",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask2")
+ subtaskResponses = append(subtaskResponses, "subtask2")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1", "subtask2"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.NoError(t, err)
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 2,
+ FinishedSubTasks: 2,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask2",
+ SubTaskNumber: 2,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_COMPLETED, response.result.Status)
+ require.Equal(t, []string{"subtask1", "subtask2"}, subtaskResponses)
+}
+
+func TestPluginRunner_twoSubtasks_secondErrorsOut(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ taskCtx.GetLogger().Info("running task in %s", pluginHelper.mock.RootPkgPath())
+ connectionId, ok := options["ConnectionId"]
+ require.True(t, ok)
+ require.Equal(t, 1.0, connectionId)
+ return nil, nil
+ }).Once()
+ var subtaskResponses []string
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ subtaskResponses = append(subtaskResponses, "subtask1")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ {
+ Name: "subtask2",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask2")
+ subtaskResponses = append(subtaskResponses, "subtask2")
+ return errors.Default.New("subtask2 error")
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1", "subtask2"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.Error(t, err)
+ require.Contains(t, err.Messages().Format(), "subtask2 error")
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 2,
+ FinishedSubTasks: 1,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask2",
+ SubTaskNumber: 2,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_FAILED, response.result.Status)
+ require.Equal(t, []string{"subtask1", "subtask2"}, subtaskResponses)
+}
+
+func TestPluginRunner_twoSubtasks_onlyRunOne(t *testing.T) {
+ pluginHelper := NewMockPluginHelper()
+ pluginHelper.PrepareTaskData(func(taskCtx core.TaskContext, options map[string]interface{}) (interface{}, errors.Error) {
+ taskCtx.GetLogger().Info("running task in %s", pluginHelper.mock.RootPkgPath())
+ connectionId, ok := options["ConnectionId"]
+ require.True(t, ok)
+ require.Equal(t, 1.0, connectionId)
+ return nil, nil
+ }).Once()
+ var subtaskResponses []string
+ pluginHelper.SubTaskMetas(func() []core.SubTaskMeta {
+ return []core.SubTaskMeta{
+ {
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask1")
+ subtaskResponses = append(subtaskResponses, "subtask1")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ {
+ Name: "subtask2",
+ EntryPoint: func(c core.SubTaskContext) errors.Error {
+ c.GetLogger().Info("inside subtask2")
+ subtaskResponses = append(subtaskResponses, "subtask2")
+ return nil
+ },
+ Required: false,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ },
+ }
+ }).Once()
+ response, err := runPlugin(t, "test_plugin", pluginHelper.GetPlugin(), &models.Task{
+ Plugin: "test_plugin",
+ Subtasks: toJSON([]string{"subtask1"}),
+ Options: toJSON(map[string]interface{}{"ConnectionId": 1}),
+ Status: models.TASK_CREATED,
+ PipelineId: 1,
+ PipelineRow: 2,
+ PipelineCol: 1,
+ })
+ require.NoError(t, err)
+ require.Equal(t, models.TaskProgressDetail{
+ TotalSubTasks: 1,
+ FinishedSubTasks: 1,
+ TotalRecords: 0,
+ FinishedRecords: 0,
+ SubTaskName: "subtask1",
+ SubTaskNumber: 1,
+ }, response.progressDetail)
+ require.Equal(t, models.TASK_COMPLETED, response.result.Status)
+ require.Equal(t, []string{"subtask1"}, subtaskResponses)
+}
+
+func toJSON(obj interface{}) datatypes.JSON {
+ b, err := json.Marshal(obj)
+ if err != nil {
+ panic(err)
+ }
+ return b
+}
+
+func runPlugin(t *testing.T, pluginName string, plugin *mocks.MockPlugin, task *models.Task) (*pluginResponse, errors.Error) {
+ ctx := context.Background()
+ tester := e2ehelper.NewDataFlowTester(t, pluginName, plugin)
+ log := tester.Log.Nested("test")
+ once.Do(func() {
+ gitlabhack.Init(config.GetConfig(), tester.Log, tester.Db) // hack until this isn't needed anymore
Review Comment:
hack to work around the issue from [here](https://github.com/apache/incubator-devlake/pull/3062#discussion_r972506767)
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
klesh commented on PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#issuecomment-1324614710
Hi, @keon94, I'm cleaning up the PRs, What should we do for this PR?
I still don't think e2e-test for `runner` is a good idea.
If you believe this should be merged, feel free to talk to other @hezyin and other dev, we can merge it if most of us agreed.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972559799
##########
plugins/core/e2e/plugin_runner_test.go:
##########
@@ -0,0 +1,108 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "context"
+ "encoding/json"
+ "github.com/apache/incubator-devlake/helpers/e2ehelper"
+ "github.com/apache/incubator-devlake/logger"
+ "github.com/apache/incubator-devlake/migration"
+ "github.com/apache/incubator-devlake/models"
+ "github.com/apache/incubator-devlake/models/migrationscripts"
+ "github.com/apache/incubator-devlake/plugins/core"
+ "github.com/apache/incubator-devlake/runner"
+ "github.com/stretchr/testify/require"
+ "gorm.io/datatypes"
+ "sync"
+ "testing"
+)
+
+func TestPluginRunner(t *testing.T) {
+ ctx := context.Background()
+ plugin := TestPlugin{}
+ plugin.AddSubTaskMetas(core.SubTaskMeta{
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) error {
+ plugin.subtasksCalled = append(plugin.subtasksCalled, "subtask1")
+ c.GetLogger().Info("inside subtask1")
+ return nil
+ },
+ Required: true,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ })
+ tester := e2ehelper.NewDataFlowTester(t, "test_plugin", &plugin)
+ log := logger.Global.Nested("test")
Review Comment:
I've made a plugins/e2e directory and put it in there.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
klesh commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r939976869
##########
plugins/core/e2e/plugin.go:
##########
@@ -0,0 +1,56 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/plugins/core"
+)
+
+type TestPlugin struct {
Review Comment:
Looks like this is for test only? please rename it to `xxx_test.go`.
##########
plugins/core/e2e/plugin_runner_test.go:
##########
@@ -0,0 +1,108 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "context"
+ "encoding/json"
+ "github.com/apache/incubator-devlake/helpers/e2ehelper"
+ "github.com/apache/incubator-devlake/logger"
+ "github.com/apache/incubator-devlake/migration"
+ "github.com/apache/incubator-devlake/models"
+ "github.com/apache/incubator-devlake/models/migrationscripts"
+ "github.com/apache/incubator-devlake/plugins/core"
+ "github.com/apache/incubator-devlake/runner"
+ "github.com/stretchr/testify/require"
+ "gorm.io/datatypes"
+ "sync"
+ "testing"
+)
+
+func TestPluginRunner(t *testing.T) {
+ ctx := context.Background()
+ plugin := TestPlugin{}
+ plugin.AddSubTaskMetas(core.SubTaskMeta{
+ Name: "subtask1",
+ EntryPoint: func(c core.SubTaskContext) error {
+ plugin.subtasksCalled = append(plugin.subtasksCalled, "subtask1")
+ c.GetLogger().Info("inside subtask1")
+ return nil
+ },
+ Required: true,
+ EnabledByDefault: true,
+ Description: "desc",
+ DomainTypes: []string{"dummy_domain"},
+ })
+ tester := e2ehelper.NewDataFlowTester(t, "test_plugin", &plugin)
+ log := logger.Global.Nested("test")
Review Comment:
depending on a `Global Variable` in a test case is no good, mock one pls.
why testing `runner` stuff inside `core` module? no, this is no no no.
please place the test case alongside the target.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972559445
##########
plugins/core/e2e/plugin.go:
##########
@@ -0,0 +1,56 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/plugins/core"
+)
+
+type TestPlugin struct {
Review Comment:
I did, but it's just a helper. No tests in it
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r972559445
##########
plugins/core/e2e/plugin.go:
##########
@@ -0,0 +1,56 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/plugins/core"
+)
+
+type TestPlugin struct {
Review Comment:
It's just a helper. No tests in it, so it won't be compiled against any actualy source file
##########
plugins/core/e2e/plugin.go:
##########
@@ -0,0 +1,56 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/plugins/core"
+)
+
+type TestPlugin struct {
Review Comment:
It's just a helper. No tests in it, so it won't be compiled against any actual source file
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#issuecomment-1248895372
e2e tests currently failing because of [this change](https://github.com/apache/incubator-devlake/pull/3085/files#diff-b11d7f47b9a12571b226171904db7192a2a9f4f620f1537553fec69c9c603dc5R36)
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#issuecomment-1249593252
> Hi, Keon, looks like you are writitest casesses for the `runner` module. Would you move them to the module? I don't think `plugins` module is the right place to be.
Hey Klesh. I can do that, but since this is an e2e test, not a unit test, it'll be unusual. That's why I added it in under plugins/.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on code in PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#discussion_r973538862
##########
test/lifecycle.go:
##########
@@ -0,0 +1,53 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package test
+
+import (
+ "github.com/apache/incubator-devlake/config"
+ "github.com/apache/incubator-devlake/helpers/e2ehelper"
+ "github.com/apache/incubator-devlake/plugins/core"
+ gitlabhack "github.com/apache/incubator-devlake/plugins/gitlab/api"
+ "github.com/apache/incubator-devlake/services"
+ "sync"
+)
+
+var once = &sync.Once{}
+
+// Setup sets up the environment for integration/e2e non-plugin tests. Wipes the DB and reruns all migrations.
+// NOTE: all plugins tests must run before any tests that use this function, otherwise there will be duplicate migration attempts!
Review Comment:
consolidated the test/api/task and runner migrations logic in one place. It's important to delete any previously generated tables first, like I am doing here, because the plugins tests manually create their own tables and than can lead to duplicate table errors when running the logic here.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 commented on pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 commented on PR #2677:
URL: https://github.com/apache/incubator-devlake/pull/2677#issuecomment-1325656964
> Hi, @keon94, I'm cleaning up the PRs, What should we do for this PR? I still don't think e2e-test for `runner` is a good idea. If you believe this should be merged, feel free to talk to other @hezyin and other dev, we can merge it if most of us agreed.
I'm ok with discarding and closing this. I have ideas about adding broader integration tests in the test/api package later.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] keon94 closed pull request #2677: [feat-2676]: e2e testing for runner.RunTask
Posted by GitBox <gi...@apache.org>.
keon94 closed pull request #2677: [feat-2676]: e2e testing for runner.RunTask
URL: https://github.com/apache/incubator-devlake/pull/2677
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org