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

Screenshots outside of "user.dir" in CI server #832

Merged
merged 10 commits into from Oct 9, 2018

Conversation

Projects
None yet
3 participants
@admizh
Copy link
Contributor

admizh commented Oct 5, 2018

Proposed changes

  1. When you use reportsFolder a located outside of "user.dir" in CI server, selenide generate wrong path in final error message.
    My changes provide additional ability to catch this, now:
config.reportsFolder("C://artifacts-storage/"); //directory, that not in 'user.dir'
config.reportsUrl=http://ci.ru/

then selenide will do simple concatenation [config.reportsUrl + screanshot/pagesource file name]

  1. Added Teamcity detection

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)

admizh and others added some commits Oct 5, 2018

1. fix reportUrl calculation for outside saved artifacts (reportUrl +…
… fileName)

2. add teamcity support
3. minor refactoring
I301235
I301235
I301235
@rosolko

This comment has been minimized.

Copy link
Collaborator

rosolko commented Oct 5, 2018

Please, run all verification and tests locally first.
./gradlew check htmlunit chrome_headless firefox_headless

Test still failing:

com.codeborne.selenide.ex.ErrorMessagesTest > convertsReportUrlForOutsideSavedScreenshot() FAILED
    java.lang.AssertionError at ErrorMessagesTest.java:76

I301235 added some commits Oct 5, 2018

I301235
I301235
I301235
@admizh

This comment has been minimized.

Copy link
Contributor

admizh commented Oct 6, 2018

Please, run all verification and tests locally first.
./gradlew check htmlunit chrome_headless firefox_headless

yes, it executed. But some issues here (in Windows). I see checkstyle failures for fresh fetched repository, without any changes:

[ant:checkstyle] [ERROR] C:\Develop\projects\selenide\selenide-fork\src\main\java\com\codeborne\selenide\webdriver\SafariDriverFactory.java:0: File does not end with a newline. [NewlineAtEndOfFile]
...
...
[ant:checkstyle] [ERROR] C:\Develop\projects\selenide\selenide-fork\src\main\java\com\codeborne\selenide\webdriver\WebDriverBinaryManager.java:0: File does not end with a newline. [NewlineAtEndOfFile]
[ant:checkstyle] [ERROR] C:\Develop\projects\selenide\selenide-fork\src\main\java\com\codeborne\selenide\webdriver\WebDriverFactory.java:0: File does not end with a newline. [NewlineAtEndOfFile]

> Task :checkstyleMain FAILED
:checkstyleMain (Thread[Task worker for ':',5,main]) completed. Took 3.648 secs.

@rosolko i think check-style tests depends of operation system, and some other jvm settings.

Test still failing:

com.codeborne.selenide.ex.ErrorMessagesTest > convertsReportUrlForOutsideSavedScreenshot() FAILED
    java.lang.AssertionError at ErrorMessagesTest.java:76

fixed in: 6315e78

@admizh

This comment has been minimized.

Copy link
Contributor

admizh commented Oct 8, 2018

@rosolko please describe your points ?

@rosolko

This comment has been minimized.

Copy link
Collaborator

rosolko commented Oct 8, 2018

@admizh CI type identifier not generic enough. It's ok for now, but we need to refactor and improve this method in future.

@rosolko rosolko removed the not sure label Oct 8, 2018

I301235 added some commits Oct 8, 2018

I301235
I301235

@asolntsev asolntsev merged commit edee096 into selenide:master Oct 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@asolntsev asolntsev added this to the 5.0.0-rc.3 milestone Oct 9, 2018

@asolntsev asolntsev self-assigned this Oct 9, 2018

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Oct 9, 2018

@admizh @rosolko Thank you! Squashed and merged. Releasing Selenide 5.0.0-rc.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment