Skip to content

Migrate tests to Junit5 + AssertJ#757

Merged
asolntsev merged 46 commits into
selenide:masterfrom
rosolko:junit5
Jul 17, 2018
Merged

Migrate tests to Junit5 + AssertJ#757
asolntsev merged 46 commits into
selenide:masterfrom
rosolko:junit5

Conversation

@rosolko
Copy link
Copy Markdown
Collaborator

@rosolko rosolko commented Jun 27, 2018

Proposed changes

Introduce BrowserStrategyExtension, ScreenShooterExtension, SoftAssertsExtension, TextReportExtension and test MockWebDriverExtension extensions as port of Junit4 rules.

Still not sure if we need to save old Junit4 test SoftAssertJUnitTest that ignored for a long time.
Also this test requred to save old LegacyIntegrationTest base test.
As solution I propose to delete this methods as test doesn't run.

Note1: Squash commits until merge.

Note2: Also I propose to move Junit4 rules and Junit5 extensions to modules. So no dependencies confuse facing.

Update: Ignored junit4 rule test replaced with documentation.

Checklist

  • Checkstyle and unit tests pass locally with my changes by running gradle check chrome htmlunit command
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

rosolko added 30 commits June 8, 2018 14:11
# Conflicts:
#	src/test/java/com/codeborne/selenide/ex/ListSizeMismatchTest.java
#	src/test/java/com/codeborne/selenide/ex/TextMismatchTest.java
#	src/test/java/com/codeborne/selenide/webdriver/RemoteDriverFactoryHeadlessOptionsTest.java
@rosolko rosolko requested review from BorisOsipov and asolntsev June 27, 2018 16:51
@BorisOsipov
Copy link
Copy Markdown
Collaborator

why do you choice AssertJ? what was wrong with hamcrest?

@vinogradoff
Copy link
Copy Markdown
Member

vinogradoff commented Jun 27, 2018 via email

/*
* Copyright 2018 Aliaksandr Rasolka
*
* Licensed under the Apache License, Version 2.0 (the "License");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Selenide is licensed by MIT. Why we have here Apache 2?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oops. Sorry about that. Copied from another project.

@rosolko
Copy link
Copy Markdown
Collaborator Author

rosolko commented Jun 27, 2018

@BorisOsipov
For first hamcrest for now development really slow.
For second hamcrest going from junit4 and in assert expected going before actual so it's really confusing.
Also assertj provide more fluent api and contains a lot of smart assertions for example - exceptions.
As it's dsl - IDE autocomplete asserts and possible asserts.
Less imports.
More natural and intuitive asserts.
Good articles related:
https://professional-practical-programmer.com/2016/06/26/assert-j/
And:
http://randomthoughtsonjavaprogramming.blogspot.com/2017/03/assertj-vs-hamcrest.html

* @author Aliaksandr Rasolka
* @since 4.12.2
*/
public class UnitTest implements WithAssertions {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand what the metaphor should stand behind this class.
In any case, it should not appear in the main package of selenide, because as far as I know some of Selenide users - there will be many of them trying to use this class in every crazy and terrible way they can find :)

rosolko added 10 commits June 28, 2018 10:10
Remove all hamcrest using.
Drop legacy integration test with junit4 rules.
Remove junit vintage engine.
Remove junit4 video recorder.
Remove UnitTest class. Use direct implements instead.
Add soft assert rule documentation.
# Conflicts:
#	src/test/java/com/codeborne/selenide/impl/DownloadFileWithProxyServerTest.java
#	src/test/java/integration/FileDownloadViaHttpGetTest.java
#	src/test/java/integration/FileDownloadViaProxyTest.java
#	src/test/java/integration/LocalHttpServer.java
#	src/test/resources/page_with_uploads.html
# Conflicts:
#	src/test/java/com/codeborne/selenide/ElementsCollectionTest.java
#	src/test/java/com/codeborne/selenide/impl/BySelectorCollectionTest.java
#	src/test/java/com/codeborne/selenide/impl/FilteringCollectionTest.java
#	src/test/java/com/codeborne/selenide/impl/HeadOfCollectionTest.java
#	src/test/java/com/codeborne/selenide/impl/LastCollectionElementTest.java
#	src/test/java/com/codeborne/selenide/impl/TailOfCollectionTest.java
#	src/test/java/integration/CollectionWaitTest.java
Comment thread build.gradle Outdated
id 'checkstyle'
id 'com.github.spotbugs' version '1.6.2'
id 'org.sonarqube' version '2.6.2'
id 'com.github.ben-manes.versions' version '0.20.0'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this for?

* @author Aliaksandr Rasolka
* @since 4.12.2
*/
public class BrowserStrategyExtension implements AfterAllCallback {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Add javadoc with usage sample.
  2. It's not clear from javadoc, is this "browser per class" or "browser per method"?


/**
* Initialize default screen shooter extension with disabled successful tests capture.
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. remove useless javadoc
  2. call this(false)

public class ScreenShooterExtension implements BeforeAllCallback, AfterEachCallback, AfterAllCallback {
private static final Logger log = Logger.getLogger(ScreenShooterExtension.class.getName());

private boolean captureSuccessfulTests;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make the field final

* @return ErrorsCollector instance
*
* @see ErrorsCollector
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove useless javadoc

* Used during extension registration.
*
* @see ErrorsCollector
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

instead of useless javadoc, please add a code sample: how to use this class?

@rosolko
Copy link
Copy Markdown
Collaborator Author

rosolko commented Jul 16, 2018

@asolntsev All issues were fixed.

@asolntsev asolntsev added this to the 4.12.3 milestone Jul 17, 2018
@asolntsev asolntsev merged commit 994c263 into selenide:master Jul 17, 2018
@rosolko rosolko deleted the junit5 branch July 18, 2018 06:26
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.

4 participants