SebastiaanKlippert / go-wkhtmltopdf

Golang commandline wrapper for wkhtmltopdf
MIT License
1.06k stars 146 forks source link

Mocking wkhtmltopdf #58

Closed tprovoost closed 3 years ago

tprovoost commented 3 years ago

Hi,

I would like to mock wkhtmltopdf for unit tests, and for that I need to mock the following functions:

AddPage(p pdf.page) error
Create() error
WriteFile(filename string) error

As you can see, page is not exported, meaning I simply can't create a mock structure. It is also a problem with documentation.

Right now the only solution I am finding would be to have another level of abstraction with PageReader. Or did I miss something?

SebastiaanKlippert commented 3 years ago

Do you have an example of what you want to achieve for me to understand it?

The reason I made the interface private is that I don't see any reason for a user to implement their own type.

Your code will not use page anywhere in the implementation directly, so I don't quite understand why you would like to mock it in a test? page is the interface, which is implemented by struct Page (and PageReader), so your will use that, by calling NewPage (or NewPageReader).

So if you create an interface in your own code for go-wkhtmltopdf, you can just use the concrete types there.

And if you really want to have your own implementation of page for test purposes you can do

type MockPage struct {}

func (m *MockPage)  Args() []string {return []string{}}
func (m *MockPage)  InputFile() string {return "www.github.com"}
func (m *MockPage)  Reader() io.Reader{ return nil}

But keep this is in your _test.go files.

tprovoost commented 3 years ago

I want to mock the PDF Generator itself, not a page.

The reason why is: the functions Create() and WriteFile() will obviously return an error if the binary is not present on the machine. I do not want to install wkhtmltopdf binary for unit testing because those are not integration tests.

func CreatePDF() error {    
        pdfg := wkhtmltopdf.NewPDFGenerator()

        page := wkhtmltopdf.NewPage("path/to/file.html")
    pdfg.AddPage(page)

        if err := pdfg.Create(); err != nil {
                // always returns error when unit testing
                return err
        }

        return pdfg.WriteFile("path/to/outputFile")
}

Now, for my unit testing I want to mock those three functions I use from your module. Meaning, I want to do that:

type PdfGenerator interface {
    AddPage(p page)
    Create() error
    WriteFile(filename string) error
}

// update the function to use the interface and not
// have a dependency on your module
func CreatePDF(pdfg PdfGenerator) error {
        ...
}

// --------
// Mocking the pdf generator
// -------
type MockPdfGenerator struct {
}

func (pdfg *MockPdfGenerator) AddPage(p page) {
        fmt.Println("added Page")
}

func (pdfg *MockPdfGenerator) Create() error {
        fmt.Println("Created")
}

// -----
// Test now has no dependency on wkhtmltopdf
func (pdfg *MockPdfGenerator) WriteFile(fileName string) error {
        fmt.Println("Written file")
}

func TestCreatePDFSuccess(t *testing.T) {
        mockPdfg := MockPdfGenerator{}

        err := myStruct.CreatePDF(mockPdfg)
        assert.NotEqual(t, nil, err)
}

// ----
// main code would then be:
func main() {
        pdfg := wkhtmltopdf.NewPDFGenerator()
        CreatePDF(pdfg)
}
SebastiaanKlippert commented 3 years ago

I want to mock the PDF Generator itself, not a page.

If you really want no dependency to go-wkhtmltopdf you would need to mock the page.

But I think my answer is still valid and you can use the Page type in your own interface.

type PdfGenerator interface {
    AddPage(p wkhtmltopdf.Page)
    Create() error
    WriteFile(filename string) error
}

Or if you don't want an import to wkhtmltopdf implement your own page interface.

tprovoost commented 3 years ago

I tried your way. If I do this, I cannot use *wkhtmltopdf.PDFGenerator anymore because it does not match the PdfGenerator interface...

Correct me if I am wrong, but for that to work, I need both wkhtmltopdf.PDFGenerator and MockPdfGenerator to have the same method signatures, which is not possible because page is not exported.

Right now, this is what I am doing:

type PdfGenerator interface {
    AddPage(p *pdf.PageReader)
    Create() error
    WriteFile(filename string) error
}

type PdfGeneratorImpl struct {
    internal *pdf.PDFGenerator
}

func NewPdfGeneratorImpl(pdfg *pdf.PDFGenerator) *PdfGeneratorImpl {
    return &PdfGeneratorImpl{
        internal: pdfg,
    }
}

func (g *PdfGeneratorImpl) AddPage(p *pdf.PageReader) {
    g.internal.AddPage(p)
}

func (g *PdfGeneratorImpl) Create() error {
    return g.internal.Create()
}

func (g *PdfGeneratorImpl) WriteFile(filename string) error {
    return g.internal.WriteFile(filename)
}

type MockPdfGenerator struct {
    ShouldFailCreate bool
    ShouldFailWrite  bool
}

func (g *MockPdfGenerator) AddPage(p *pdf.PageReader) {
    // do nothing
}

func (g *MockPdfGenerator) Create() error {
    if g.ShouldFailCreate {
        return errors.New("Failed to create pdf")
    }

    return nil
}

func (g *MockPdfGenerator) WriteFile(filename string) error {
    if g.ShouldFailWrite {
        return errors.New("Failed to write pdf")
    }

    return nil
}

And it would be much simpler to remove all the internal part, which is there because I cannot do:

type PdfGenerator interface {
    AddPage(p page)
    Create() error
    WriteFile(filename string) error
}
SebastiaanKlippert commented 3 years ago

I understand, thanks for the code. I assumed you already had an implentation of PdfGeneratorImpl in your code which used the PageReader type, but you are mocking out the actual pdf generator. I rarely do that to be honest, but I see what you want to do.

I can take a look when I have more time to see if I can find a workaround or can change the interface to an exported one.

For now, I think your workaround is good, but I understand it would be easier without the internal implementation.

SebastiaanKlippert commented 3 years ago

Renamed to PageProvider to make it exported. Thanks for the clear code examples and explanation. It is tagged as v1.6.1