[plugin-web-app-playwright] Implement Cookie Steps code logic for Playwright #5127

yauhenhapan · 2024-06-14T08:16:47Z

implement code logic for Playwright's cookie
write corresponding tests
write documentation

valfirst · 2024-06-21T14:10:45Z

...ugin-web-app-playwright/src/main/java/org/vividus/ui/web/playwright/helper/CookieHelper.java

+import org.slf4j.LoggerFactory;
+import org.vividus.ui.web.playwright.BrowserContextProvider;
+
+public class CookieHelper


Suggested change

public class CookieHelper

public class CookieManager

does it make sense to implement ICookieManager ?

the name was changed to CookieManager

implemented ICookieManager

valfirst · 2024-06-21T14:11:19Z

docs/modules/plugins/partials/web-cookie-steps.adoc

+==== Set cookies
+
+Adds the cookies provided in the input xref:ROOT:glossary.adoc#_examplestable[ExamplesTable]. It's allowed to add the
+cookies for the current domain only: make sure the web browser is opened at the


is it true for Playwright?

checked that Playwright stores all together, so I removed the sentence It's allowed to add the cookies for the current domain only:...

valfirst · 2024-06-21T14:12:51Z

...ugin-web-app-playwright/src/main/java/org/vividus/ui/web/playwright/helper/CookieHelper.java

+ .setPath(path)
+ .setDomain(domain);
+ LOGGER.debug("Adding cookie: {}", cookie);
+ addCookies(Collections.singletonList(cookie));


Suggested change

addCookies(Collections.singletonList(cookie));

addCookies(List.of(cookie));

refactored, thanks

valfirst · 2024-06-21T14:13:36Z

...ugin-web-app-playwright/src/main/java/org/vividus/ui/web/playwright/helper/CookieHelper.java

+ String host = URI.create(url).getHost();
+ String domain = host.contains(DOMAIN_PARTS_SEPARATOR) && !InetAddresses.isInetAddress(host)
+ ? DOMAIN_PARTS_SEPARATOR + getTopDomain(host)
+ : host;


please avoid copy-pasting the logic, it's rather recommended to use common logic to common place and re-use in both plugins

moved this common logic in DomainManager class in vividus-extension-web-app package

valfirst · 2024-06-21T14:14:54Z

...ugin-web-app-playwright/src/main/java/org/vividus/ui/web/playwright/helper/CookieHelper.java

+
+ public Cookie getCookie(String name)
+ {
+ for (Cookie cookie : getBrowserContext().cookies())


use Java Streams API

refactored, thanks

valfirst · 2024-06-21T14:18:30Z

...web-app-playwright/src/test/java/org/vividus/ui/web/playwright/action/CookieHelperTests.java

+ private static final String COOKIE_VALUE = "cookieValue";
+ private static final String DOMAIN = "https://www.domain.com";
+
+ @InjectMocks private CookieHelper cookieHelper;


it should go after all fields of type @Mock

done, thanks

valfirst · 2024-06-21T14:18:59Z

...web-app-playwright/src/test/java/org/vividus/ui/web/playwright/action/CookieHelperTests.java

+
+ @InjectMocks private CookieHelper cookieHelper;
+ @Mock private BrowserContextProvider browserContextProvider;
+ @Captor private ArgumentCaptor<BrowserContext.ClearCookiesOptions> clearCookiesOptionsCaptor;


I believe it's used once and thus it can be inlined

refactored, thanks

valfirst · 2024-06-21T14:19:15Z

...web-app-playwright/src/test/java/org/vividus/ui/web/playwright/action/CookieHelperTests.java

+ @InjectMocks private CookieHelper cookieHelper;
+ @Mock private BrowserContextProvider browserContextProvider;
+ @Captor private ArgumentCaptor<BrowserContext.ClearCookiesOptions> clearCookiesOptionsCaptor;
+ @Captor private ArgumentCaptor<List<Cookie>> cookieCaptor;


I believe it's used once and thus it can be inlined

refactored, thanks

valfirst · 2024-06-21T14:19:48Z

...n-web-app-playwright/src/test/java/org/vividus/ui/web/playwright/steps/CookieStepsTests.java

+ @Mock private CookieHelper cookieHelper;
+ @Mock private VariableContext variableContext;
+ @Mock private ISoftAssert softAssert;
+ @InjectMocks private CookieSteps cookieSteps;


it should go after all fields of type @Mock

done, thanks

valfirst · 2024-06-21T14:20:40Z

vividus-tests/src/main/resources/story/integration/CookieStepsPlaywright.story

it's not needed, I believe it is equal to the same story used for Selenium-based steps

i removed this file since we had discussed it's unnecessary right now

codecov · 2024-06-26T08:08:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.43%. Comparing base (8dc4a57) to head (43fb77f).
Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5127      +/-   ##
============================================
- Coverage     97.43%   97.43%   -0.01%     
- Complexity     6747     6999     +252     
============================================
  Files           933      936       +3     
  Lines         19520    19580      +60     
  Branches       1298     1299       +1     
============================================
+ Hits          19020    19078      +58     
- Misses          391      392       +1     
- Partials        109      110       +1

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

valfirst · 2024-06-26T10:52:01Z

vividus-extension-web-app/src/main/java/org/vividus/ui/web/action/DomainManager.java

+import com.google.common.net.InetAddresses;
+import com.google.common.net.InternetDomainName;
+
+public final class DomainManager


Suggested change

public final class DomainManager

public final class InetAddressUtils

renamed, thanks

valfirst · 2024-06-26T10:52:19Z

...dus-extension-web-app/src/test/java/org/vividus/ui/web/configuration/DomainManagerTests.java

+import org.junit.jupiter.params.provider.CsvSource;
+import org.vividus.ui.web.action.DomainManager;
+
+public class DomainManagerTests


Suggested change

public class DomainManagerTests

class DomainManagerTests

removed, thanks

valfirst · 2024-06-26T10:52:42Z

...dus-extension-web-app/src/test/java/org/vividus/ui/web/configuration/DomainManagerTests.java

+ "http://127.0.0.1:8080, 127.0.0.1",
+ "http://localhost:8080, localhost"
+ })
+ void shouldAddCookie(String urlAsString, String domain)


please update method name to reflect the actual test

updated, thanks

valfirst · 2024-06-26T10:54:21Z

...in-web-app-playwright/src/main/java/org/vividus/ui/web/playwright/action/ICookieManager.java

+ * limitations under the License.
+ */
+
+package org.vividus.ui.web.playwright.action;


move to vividus-extension-web-app and reuse in both vividus-plugin-web-app and vividus-plugin-web-app-plyawright

valfirst · 2024-06-26T10:55:01Z

...gin-web-app-playwright/src/main/java/org/vividus/ui/web/playwright/action/CookieManager.java

+ }
+
+ @Override
+ public String getValue(Cookie cookie)


why is it needed?

removed this method

valfirst · 2024-06-26T10:57:22Z

...eb-app-playwright/src/test/java/org/vividus/ui/web/playwright/action/CookieManagerTests.java

+import org.vividus.ui.web.playwright.BrowserContextProvider;
+
+@ExtendWith(MockitoExtension.class)
+public class CookieManagerTests


Suggested change

public class CookieManagerTests

class CookieManagerTests

removed, thanks

valfirst · 2024-06-26T10:57:37Z

...n-web-app-playwright/src/test/java/org/vividus/ui/web/playwright/steps/CookieStepsTests.java

+import org.vividus.variable.VariableScope;
+
+@ExtendWith(MockitoExtension.class)
+public class CookieStepsTests


Suggested change

public class CookieStepsTests

class CookieStepsTests

removed, thanks

valfirst · 2024-06-27T08:23:16Z

vividus-extension-web-app/src/main/java/org/vividus/ui/web/action/IBaseCookieManager.java

+
+ T getCookie(String cookieName);
+
+ S getCookies();


Suggested change

S getCookies();

Collection<T> getCookies();

?

fixed, thanks

valfirst · 2024-06-27T08:24:10Z

vividus-extension-web-app/src/main/java/org/vividus/ui/web/action/IBaseCookieManager.java

+
+package org.vividus.ui.web.action;
+
+public interface IBaseCookieManager<T, S>


Suggested change

public interface IBaseCookieManager<T, S>

public interface CookieManager<T, S>

(feel free to rename current classes to PlaywrightCookieManager and WebDriverCookieManager)

updated, thanks

valfirst · 2024-06-27T08:25:47Z

vividus-plugin-web-app/src/main/java/org/vividus/ui/web/action/ICookieManager.java

- Cookie getCookie(String cookieName);
-
- Set<Cookie> getCookies();
-
 CookieStore getCookiesAsHttpCookieStore();


let's implement this method for playwright cookie manager as well

valfirst · 2024-06-28T07:17:10Z

vividus-extension-web-app/src/main/java/org/vividus/ui/web/action/ICookieManager.java


-public interface ICookieManager
+public interface ICookieManager<T>


Suggested change

public interface ICookieManager<T>

public interface CookieManager<T>

renamed, thanks

yauhenhapan requested a review from a team as a code owner June 14, 2024 08:16

yauhenhapan requested review from valfirst, vkepin and uarlouski June 14, 2024 08:16

yauhenhapan force-pushed the feature/implement_cookie_steps branch 3 times, most recently from 8847e38 to 54d6034 Compare June 14, 2024 12:25

yauhenhapan force-pushed the feature/implement_cookie_steps branch 3 times, most recently from 938a635 to 1bb6f04 Compare June 21, 2024 13:08

valfirst force-pushed the feature/implement_cookie_steps branch from 1bb6f04 to 6de531b Compare June 21, 2024 13:52

valfirst requested changes Jun 21, 2024

View reviewed changes

yauhenhapan force-pushed the feature/implement_cookie_steps branch 4 times, most recently from 2816c96 to 8426539 Compare June 26, 2024 07:32

valfirst requested changes Jun 26, 2024

View reviewed changes

yauhenhapan force-pushed the feature/implement_cookie_steps branch from 8426539 to a6b66d6 Compare June 26, 2024 17:43

valfirst requested changes Jun 27, 2024

View reviewed changes

yauhenhapan force-pushed the feature/implement_cookie_steps branch 2 times, most recently from 1ba9122 to 8a760f5 Compare June 27, 2024 15:09

valfirst reviewed Jun 28, 2024

View reviewed changes

Implement Cookie Steps code logic for Playwright

43fb77f

yauhenhapan force-pushed the feature/implement_cookie_steps branch from 8a760f5 to 43fb77f Compare June 28, 2024 07:58

valfirst approved these changes Jun 28, 2024

View reviewed changes

valfirst merged commit 7f5bd5a into vividus-framework:master Jun 28, 2024
8 checks passed

	addCookies(Collections.singletonList(cookie));
	addCookies(List.of(cookie));

	public final class DomainManager
	public final class InetAddressUtils


		package org.vividus.ui.web.action;

		public interface IBaseCookieManager<T, S>

	public interface IBaseCookieManager<T, S>
	public interface CookieManager<T, S>


		public interface ICookieManager
		public interface ICookieManager<T>

	public interface ICookieManager<T>
	public interface CookieManager<T>

[plugin-web-app-playwright] Implement Cookie Steps code logic for Playwright #5127

[plugin-web-app-playwright] Implement Cookie Steps code logic for Playwright #5127

Conversation

yauhenhapan commented Jun 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 26, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 26, 2024 •

edited

Loading