playwright-go icon indicating copy to clipboard operation
playwright-go copied to clipboard

[PoC] feat: add web first assertions (#271)

Open masaushi opened this issue 3 years ago • 3 comments

This is an implementation for web first assertions (#271).

An example of usage is bellow,

assertions := playwright.NewPlaywrightAssertions()

err := assertions.ExpectLocator(locator).ToHaveText("example")
if err != nil {
    // handle error
}

The current implementation still works, but I think there are some areas for improvements. Belows are some of my considerations.

  • PlaywrightAssertions interface is possibly unneeded. It may be simpler to call Expect function directory instead of through PlaywrightAssertions interface.
// example

err := playwright.ExpectLocator(locator).ToHaveText("example")
  • I'm not sure it's more useful to be integrated with *testing.T. Current implementation returns error to handle it by ourselves.
// example

assertions := playwright.NewPlaywrightAssertions(t) // `t` is `*testing.T`
// no return value, error is reported in `ToHaveText` method
assertions.ExpectLocator(locator).ToHaveText("example")

There may be other improvements other than the above, so please let me know if anyone has some good ideas!

masaushi avatar Jun 05 '22 06:06 masaushi

Hi @masaushi ! Good work!

PlaywrightAssertions interface is possibly unneeded. It may be simpler to call Expect function directory instead of through PlaywrightAssertions interface.

I agree with you.

I'm not sure it's more useful to be integrated with *testing.T.

I think we need to think about that. In my personal opinion, there may be pitfalls. Therefore, I propose to raise this issue separately.

And I have a few additional questions.

  1. Have you considered the following syntax? Since in golang we do not have methods overloading we can slightly change the methods that return assertion. I suggest this use for page, locator and possibly in the future for apiResponse and screenshot. But your way is also good.
// Locator
err := Expect.Locator(locator).ToHaveText("example")
// Page
err = Expect.Page(page).Not().ToHaveTitle("Simple Domain")
...
  1. As you can see from my example above, I propose to make the "Not" method public. This would allow us to make the "assertions" package more compact while maintaining flexibility of use. It also supports by upstream Docs

The both suggestions is my opinion, it would be great to know what the community think about these moments.

sttaran avatar Jun 06 '22 08:06 sttaran

Thank you @StanislavTaran for your great review!

Have you considered the following syntax? Since in golang we do not have methods overloading we can slightly change the methods that return assertion. I suggest this use for page, locator and possibly in the future for apiResponse and screenshot. But your way is also good.

Your way is great! From your way, I come up with a similar idea that is to have expect package for syntax sugar of current assertions implementation.

// in the expect package

package expect

import "github.com/playwright-community/playwright-go"

var Locator playwright.LocatorAssertions
var Page playwright.PageAssertions

func init() {
    // initialize Locator and Page variables
}
// usage example

import "github.com/playwright-community/playwright-go/expect"

func TestSomething(t *testing.T) {

    // something about locator

    err := expect.Locator(locator).ToHaveText("example")
}

But this is just an idea.


I propose to make the "Not" method public. This would allow us to make the "assertions" package more compact while maintaining flexibility of use.

It's good to have public Not() methods! As you say, having public Not() method seems better than having Not~ methods of each assertion methods for the future maintaining.

masaushi avatar Jun 19 '22 07:06 masaushi

Your way is great! From your way, I come up with a similar idea that is to have expect package for syntax sugar of current assertions implementation.

@masaushi Thanks! Actually, I meant something like this for assertions.

assertions.go

// declaration
package playwright

type expect struct {
}

var Expect expect

func (e *expect) Page(page Page) PageAssertions {
	return newPageAssertions(page, Bool(false))   // <---- isNot Option = false
}

func (e *expect) Locator(locator Locator) LocatorAssertions {
	return newLocatorAssertions(locator, Bool(false))  // <---- isNot Option = false
}

assertions__test.go

// usage
err = playwright.Expect.Page(page).ToHaveTitle("Example Domain", playwright.PageExpectOptions{Timeout: 
playwright.Float(1000)})
require.NoError(t, err)

err = playwright.Expect.Page(page).Not().ToHaveTitle("Simple Domain", playwright.PageExpectOptions{Timeout: 
playwright.Float(1000)})
require.NoError(t, err)

This is very similar to your first version. It's not a silver bullet, but I think it looks good

sttaran avatar Jun 20 '22 13:06 sttaran