-
Notifications
You must be signed in to change notification settings - Fork 44
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
test: implement tasktype and state processor tests #998
Conversation
@@ -1,62 +0,0 @@ | |||
package processor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer needed, replaced with processor.New
@@ -46,6 +88,25 @@ type ReportProcessor interface { | |||
|
|||
var log = logging.Logger("lily/index/processor") | |||
|
|||
const BuiltinTaskName = "builtin" | |||
|
|||
func New(api tasks.DataSource, name string, taskNames []string) (*StateProcessor, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaces processor.Builder
ReportProcessors map[string]ReportProcessor | ||
} | ||
|
||
func MakeProcessors(api tasks.DataSource, indexerTasks []string) (*IndexerProcessors, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was moved from tipset.go
, mechanical change with tests for the method in state_test.go
"github.com/filecoin-project/lily/tasks/msapprovals" | ||
) | ||
|
||
func TestNewProcessor(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ensure a new processor from all tasks has the expected processors registered. Will prevent issues like: #996
"github.com/filecoin-project/lily/tasks/msapprovals" | ||
) | ||
|
||
func TestMakeProcessorsActors(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
granular testing of the MakeProcessors
method.
require.Len(t, proc.ActorProcessors, 21) | ||
require.Len(t, proc.TipsetProcessors, 5) | ||
require.Len(t, proc.TipsetsProcessors, 9) | ||
require.Len(t, proc.ReportProcessors, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serves as a reminder to implementers to update this test file when/if a new processor is added.
Codecov Report
@@ Coverage Diff @@
## master #998 +/- ##
========================================
+ Coverage 35.3% 36.8% +1.5%
========================================
Files 31 33 +2
Lines 2090 2438 +348
========================================
+ Hits 739 899 +160
- Misses 1266 1452 +186
- Partials 85 87 +2 |
} | ||
|
||
func (ti *TipSetIndexer) init() error { | ||
var indexerTasks []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to tasktype package
} | ||
|
||
for _, t := range indexerTasks { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to processor package.
const TotalTableTasks = 35 | ||
|
||
func TestMakeAllTaskNames(t *testing.T) { | ||
actual, err := tasktype.MakeTaskNames(tasktype.AllTableTasks) | ||
require.NoError(t, err) | ||
// if this test fails it means a new task name was added, update the above test | ||
require.Len(t, actual, TotalTableTasks) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serves as a reminder to implementers to update this test file when new tasks are added.
TipsetsProcessors map[string]TipSetsProcessor | ||
ActorProcessors map[string]ActorProcessor | ||
ReportProcessors map[string]ReportProcessor | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It's a smell to have public fields within the package if they are not being consumed publicly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are used (inspected for assertions) in the processor_test
package.
New
method, simplifying calling code.MakeProcessors
method accepting a slice of granular task names and returning the required processors for executing them.MakeProcessors
.MakeTaskNames
method accepting a slice of tasks from the user are returning the granular task names for themMakeTaskNames