playwright-go
playwright-go copied to clipboard
[PoC] feat: add web first assertions (#271)
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!
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.
- 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")
...
- 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.
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.
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