Skip to content

Commit

Permalink
Don't initialize rfbrowser
Browse files Browse the repository at this point in the history
  • Loading branch information
davisagli committed Sep 5, 2024
1 parent fb5b939 commit f1a01b8
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ deps =
# constraints_file = "https://my-server.com/constraints.txt"
##
commands =
rfbrowser init
pytest --disable-warnings {posargs} {toxinidir}/tests
extras =
test
Expand Down Expand Up @@ -166,7 +165,6 @@ deps =
-c constraints-mxdev.txt

commands =
rfbrowser init
coverage run --source plone.volto -m pytest {posargs} --disable-warnings {toxinidir}/tests
coverage report -m --format markdown
coverage xml
Expand Down

2 comments on commit f1a01b8

@mauritsvanrees
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove rfbrowser init? Sure, it takes time, but the next time someone runs plone/meta it will get added again, because we have plone.app.robotframework in setup.py, which serves as its cue to add these lines.

I tried with and without these lines, and in both cases 84 tests are run. So it should not matter.
But if I understand the purpose correctly, rfbrowser init makes sure that Chrome and the chromedriver are in sync, otherwise the robot tests may not run when we try again in a few weeks or so. See for example plone/jenkins.plone.org#362, although apparently in that case rfbrowser init got run but did not help... Maybe it helps in most cases, but not always.

@davisagli
Copy link
Member Author

Choose a reason for hiding this comment

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

@mauritsvanrees I removed it because this package doesn't have any robotframework tests, so it's not really needed. But it does provide an acceptance layer which depends on robotframework, so it has to be in setup.py. I agree it doesn't matter much whether it is here or not.

Please sign in to comment.