Skip to content
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

Add browser autofill #1183

Merged
merged 7 commits into from
Jan 12, 2024
Merged

Conversation

faridnsh
Copy link
Contributor

@faridnsh faridnsh commented Dec 14, 2023

Fixes #1182

We could make the code a little bit smarter, or even have the field css selectors(username, password, submit button) configured from config file instead.

@faridnsh faridnsh force-pushed the feature/browser-autofill branch from 7fe33f0 to 2700881 Compare December 14, 2023 10:55
@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2023

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (b1ae9e3) 39.72% compared to head (5e13ec1) 39.74%.

Files Patch % Lines
pkg/provider/browser/browser.go 48.48% 13 Missing and 4 partials ⚠️
pkg/flags/flags.go 0.00% 2 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1183      +/-   ##
==========================================
+ Coverage   39.72%   39.74%   +0.02%     
==========================================
  Files          53       53              
  Lines        8027     8063      +36     
==========================================
+ Hits         3189     3205      +16     
- Misses       4411     4426      +15     
- Partials      427      432       +5     
Flag Coverage Δ
unittests 39.74% <44.44%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@faridnsh faridnsh force-pushed the feature/browser-autofill branch from 2700881 to 04948a3 Compare December 14, 2023 11:07
@tinaboyce
Copy link
Contributor

Hi @alFReD-NSH ,

Thanks for doing a PR. Are you able to provide tests for this?

@faridnsh
Copy link
Contributor Author

There's no tests currently for the browser provider, I'm not sure what's the reason for it, but I'm guessing it's because it's not as easy as other providers. I would appreciate if some directions or tips could be provided. I'll give it a try in couple of weeks.

@mapkon
Copy link
Contributor

mapkon commented Dec 19, 2023

There's no tests currently for the browser provider, I'm not sure what's the reason for it, but I'm guessing it's because it's not as easy as other providers. I would appreciate if some directions or tips could be provided. I'll give it a try in couple of weeks.

Can you reference browser_test.go?

@faridnsh
Copy link
Contributor Author

faridnsh commented Jan 9, 2024

@mapkon

I did indeed have a look at it and the actual part that runs the browser provider for a successful outcome is commented out(line 159). I tried to write a similar test with the mocked page type that already exists and I get the following error:

pkg/provider/browser/browser_test.go:249:18: cannot use page (variable of type *mocks.Page) as playwright.Page value in argument to autoFill: *mocks.Page does not implement playwright.Page (wrong type for method ExposeFunction)
                have ExposeFunction(string, func([]interface{}) interface{}) error
                want ExposeFunction(string, func(...interface{}) interface{}) error

I tried regenerating the mocks, but didn't work, Mockery doesn't detect the difference. This could be fixable by Mockery, so I'll report an issue there.

@faridnsh
Copy link
Contributor Author

faridnsh commented Jan 9, 2024

Apparently it's a known issue and they have a PR for it: vektra/mockery#719

@mapkon
Copy link
Contributor

mapkon commented Jan 10, 2024

OK - I will review this and merge it

@tinaboyce
Copy link
Contributor

Hi all (paging @mapkon), I have updated the PR to include a test. I have managed to write test utilising a server and running the autoFill function against a hosted example form page.

Copy link
Contributor

@mapkon mapkon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is brilliant @tinaboyce - I will take a look later

@mapkon mapkon merged commit 08956d5 into Versent:master Jan 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browser autofill
4 participants