Add unit tests for moderately complex APIs across the code base

[justicezyx]

The lack of unit tests greatly slows down the pace to understand, discuss, and modify the code. I think the lack of unit tests blocks my work of integrating Glow with Kubernets: I have little idea of how code suppose to work, and there is virtually no check on the correctness of changes. It also greatly increase the effort to understand the code base.

I think it makes sense to add unit tests to cover the relatively obvious/common use cases, which should be able to finish quickly and brings a lot of benefits.

I plan to go through the code base, and add test along the way. Hopefully it also accelerate my rate of ramping up with the code base.

Any thoughts, comments, or objections?

[justicezyx]

Chris, WDYT? Please feel free to assign this issue to me if you are OK with it.

[chrislusf]

Yes, please add any unit tests whenever you see fit.

Invited you to the collaborator list. After that I can assign this issue to you.

Just ask me anything you are not familiar with.

[justicezyx]

I apologize for the build failure. I am fixing them.

There are 2 issues:

1. Lists are not sorted, causing list comparison sometime fails the channel_util_test.go.
2. Data race in network_util_test.go.

It should be straightforward to fix. But if you prefer to rollback the change, that's also fine to me.

[justicezyx]

Hi Chris, the following test code does not write the input string to the 'text' file. It seems Slice() does not behave the same way as Source() when writing output, is this expected?

  1 package flow                                                                                                             
  2                                                                                                                          
  3 import "testing"                                                                                                         
  4                                                                                                                          
  5 func TestSource(t *testing.T) {                                                                                          
  6   fc := New()                                                                                                            
  7                                                                                                                          
  8   inputChan := make(chan string, 0)                                                                                      
  9   go func() {                                                                                                            
 10     inputChan <- "test1"                                                                                                 
 11     inputChan <- "test2"                                                                                                 
 12     close(inputChan)                                                                                                     
 13   }()                                                                                                                    
 14   fc.Channel(inputChan).SaveTextToFile("test")                                                                           
 15 }

[chrislusf]

Need a map step. The implementation assumes there is a at least one map reduce step in the flow.

This test adds a simple map step.

package flow

import (
    "testing"
)

func TestSource(t *testing.T) {
    fc := New()

    inputChan := make(chan string, 0)
    go func() {
        inputChan <- "test1"
        inputChan <- "test2"
        close(inputChan)
    }()
    fc.Channel(inputChan).Map(func(t string) string {
        return t
    }).SaveTextToFile("test")

}

[justicezyx]

Thanks Chris for the explanation.

Pardon my ignorance, could you provide some clues on the difference between FlowContext.Channel() and FlowContext.Source()? Reading the source code, I could not figure out why their behaviors differ. I guess the Task/Step setups are different between them, but could not figure out the exact places.

[chrislusf]

For Channel(), the data actually flow into the system in doChannel() via ExternalInputChans. It does not have any goroutine to pump data in.

This is different from Source(), where the data come in via a function f. The Source() has goroutine to pump data in.

[justicezyx]

I see, thanks a lot for the explanation.