Removes SeleniumLibrary Firefox profile #885
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The SeleniumLibrary default Firefox profile is now removed. When Firefox created and if Firefox profile is not defined, then the default Selenium FirefoxProfile is used. Fixes robotframework#883
aaltat
force-pushed
the
use_selenium_ff_profile
branch
from
99f0729
to
5e8bad3
Compare
pekkaklarck
approved these changes
Sep 5, 2017
| logdir = os.path.dirname(logfile) | ||
| else: | ||
| logdir = BuiltIn().get_variable_value('${OUTPUTDIR}') | ||
| return logdir |
There was a problem hiding this comment.
- It took me a little time to make sure
RobotNotRunningErrorcannot occur when querying${OUTPUTDIR}. I think the code should be reorganized a little. - There seems to be a logic bug when you actually get that
RobotNotRunningError. The returned directory seems to be the parent directory of the cwd in that case. os.getcwd()returns bytes on Python 2. This causes problems if the path contains non-ASCII characters. Should either useos.getcwdu()(which doesn't exist on Python 3) or just return'.'.
Perhaps the code could be something like this:
try:
logfile = BuiltIn().get_variable_value('${LOG FILE}')
if logfile == 'NONE':
return BuiltIn().get_variable_value('${OUTPUTDIR}')
return os.path.dirname(logfile)
except RobotNotRunningError:
return os.getcwd() if PY3 else os.getcwdu()| SeleniumVersion = namedtuple('SeleniumVersion', 'major minor micro') | ||
| version = selenium.__version__.split('.') | ||
| selenium_version = SeleniumVersion(major=version[0], minor=version[1], | ||
| micro=version[2]) |
There was a problem hiding this comment.
- Copyrights missing from the file.
- +1 for namedtuple.
- Like you commented elsewhere, SELENIUM_VERSION is better name for the constant.
- Can we be certain that version always has three components? If not, this could be used:
If earlier releases have always contained three components, the above is an overkill.
major, minor, micro = (selenium.__version__.split('.') + ['0', '0'])[:3] SELENIUM_VERSION = SeleniumVersion(major, minor, micro)
There was a problem hiding this comment.
Looked releases all to they to the 2.50.0 and there always has been three components, in the version field. But in one day, there might not be, so why risk it.
Others done.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The SeleniumLibrary default Firefox profile is now removed. When
Firefox created and if Firefox profile is not defined, then the
default Selenium FirefoxProfile is used.
Fixes #883