bonigarcia / selenium-webdriver-java

Examples of the O'Reilly book "Hands-On Selenium WebDriver with Java"
https://oreil.ly/1E7CX
Apache License 2.0
172 stars 95 forks source link

The output files from test classes are written in the parent project directory, instead of the sub-project directory, which is odd #766

Closed kazurayam closed 9 months ago

kazurayam commented 10 months ago

Problem to solve

I cloned the v1.3.0, I executed

$ cd ~/tmp/selenium-webdriver-java
$ tree -L 1 .
.
├── LICENSE
├── README.md
├── alternatives
├── build
├── docs
├── gradle
├── gradlew
├── gradlew.bat
├── pom.xml
├── selenium-ide
├── selenium-webdriver-junit4
├── selenium-webdriver-junit5
├── selenium-webdriver-junit5-seljup
├── selenium-webdriver-testng
└── settings.gradle
10 directories, 6 files

$ ./gradlew :selenium-webdriver-junit4:test 
... (minutes passed)

$ tree -L 1 .
.
├── 2023.11.26_18.36.26.959-6a52be7cc3674308ba99b3523fc8dc12.png
├── 2023.11.26_18.36.28.591-6a52be7cc3674308ba99b3523fc8dc12.png
├── LICENSE
├── README.md
├── alternatives
├── build
├── docs
├── extentReport.html
├── fullpage-screenshot-chrome.png
├── fullpage-screenshot-firefox.png
├── gradle
├── gradlew
├── gradlew.bat
├── my-pdf.pdf
├── pom.xml
├── screenshot.png
├── selenium-ide
├── selenium-webdriver-junit4
├── selenium-webdriver-junit5
├── selenium-webdriver-junit5-seljup
├── selenium-webdriver-testng
├── settings.gradle
├── webdrivermanager.pdf
├── webdrivermanager.png
└── webelement-screenshot.png

10 directories, 16 files

In the parent project directory, 10 files were newly created.

Why these files were written into the parent project selenium-webdriver-java directory? Why not these files were written into the sub-project selenium-webdriver-junit4 directory?

My research

I looked at io.github.bonigarcia.webdriver.jupiter.ch04.screenshots.ScreeshotPngJunit4Test, it has the following statement:

        Path destination = Paths.get("screenshot.png");

The file screenshot.png will be created in the current working directory. Therefore, I can presume, when the test case ran, a call to System.getProperty("user.dir") returned the Path value of the parent project selenium-webdriver-java directory, not the sub-project directory selenium-webdriver-junit4.

Now I can restate my question. Why the current working directory for the test cases were set to be the parent project selenium-webdriver-java directory, not the sub-project directory selenium-webdriver-junit4? which seems very strange for me.

What I found

In the https://github.com/bonigarcia/selenium-webdriver-java/blob/1.3.0/selenium-webdriver-junit5/build.gradle, I found a line:

test {
    ...
    systemProperty System.properties
}

This single line is the cause of the issue that surprised me.

I executed the following command in the Terminal

$ pwd
~/tmp/selenium-webdriver-java
$ ./gradlew :selenium-webdriver-junit4:test

At this instant, the System.properties included the user.dir property which had the value of ~/tmp/selenium-webdriver-java. This value was passed to the test target of the selenium-webdriver-junit4/build.gradle. Therefore all JUnit4 tests found a call to System.getProperty("user.dir") was returning ~/tmp/selenium-webdriver-java, not ~/tmp/selenium-webdriver-java/selenium-webdriver-junit4.

My proposal

The JUnit4/5/TestNG-based test classes should not call java.nio.file.Paths.get(String fileName) to locate output files because Paths.get(String relativePath) depends on the value of current working directory which is not necessarily reliable. The current working directory is dependent on the runtime environment (Gradle, Maven, IntelliJ IDEA, etc).

I would recommend to resolve the project directory by the classpath. I have proposed my solution for this as follows

bonigarcia commented 9 months ago

@kazurayam Thanks a lot for your effort in improving this repo. I agree that leaving output artifacts from the test file is not an ideal approach for an actual test suite. Indeed, it would be better for the test to clean the house after completing their task. However, these tests have been developed in the context of a book to explain the Selenium API. This way, each test is devoted to showcasing a giving feature to someone trying to learn. For instance, when taking a screenshot, it would be easy for each test to remove the screenshot file after the test. But in that case, the learner can be lost since the screenshot file is not finally available (and it is the manual proof that the test is working). And for that reason, I decided to keep these output files. For this reason, I will not merge #748, but thanks again for your time.

kazurayam commented 9 months ago

@bonigarcia

Thank you for having a look at my post.

I will not merge #748,

OK. I accept that.

these tests have been developed in the context of a book to explain the Selenium API.

I understand this. These tests should be in sync with the O'Reilly book which is already printed and published.

bonigarcia commented 9 months ago

These tests should be in sync with the O'Reilly book which is already printed and published.

I try to get all the examples working. For that reason, some parts of the code need to evolve. I aim to publish a second edition of this book someday (if O'Reilly also wants). At that time, the printed/published tests in the book will be updated accordingly.