#875 FIX #894
#875 FIX #894
Conversation
|
@Tenit2012 @Trucnt you can check out these changes too. |
| int threshold = (int) Math.min(Runtime.getRuntime().freeMemory() / 10, Integer.MAX_VALUE); | ||
| FileBackedOutputStream os = new FileBackedOutputStream(threshold); | ||
| try ( | ||
| CountingOutputStream counter = new CountingOutputStream(os); |
There was a problem hiding this comment.
the formatting here is a bit confusing
| return getPrivateFieldValue("client", HttpClient.class); | ||
| } | ||
|
|
||
| private Response createSession(Command command) throws IOException { |
There was a problem hiding this comment.
it would be handy to add some documentation on this method
| @@ -125,7 +219,12 @@ public Response execute(Command command) throws WebDriverException { | |||
|
|
|||
| Response response; | |||
| try { | |||
| response = super.execute(command); | |||
| if (!NEW_SESSION.equals(command.getName())) { | |||
There was a problem hiding this comment.
I'd rather use ternary operator:
response = NEW_SESSION.equals(command.getName()) ? createSession(command) : super.execute(command)
|
|
||
|
|
||
| public class NewSessionPayload implements Closeable { | ||
| class NewAppiumSessionPayload implements Closeable { |
There was a problem hiding this comment.
Codacy does not appreciate default access modifiers
| @@ -54,11 +60,17 @@ | |||
| * The starting. | |||
| */ | |||
| @BeforeClass public static void beforeClass() { | |||
There was a problem hiding this comment.
we could keep both tests for Firefox and Chrome drivers
| Optional<Result> result = (Optional<Result>) createSessionMethod | ||
| .invoke(this, client, contentStream, counter.getCount()); | ||
|
|
||
| if (result.isPresent()) { |
There was a problem hiding this comment.
return result.map(x -> { .... }).orElseThrow(() -> new SessionNotCreatedException(...))
| Capabilities desired = (Capabilities) command.getParameters().get("desiredCapabilities"); | ||
| desired = desired == null ? new ImmutableCapabilities() : desired; | ||
|
|
||
| int threshold = (int) Math.min(Runtime.getRuntime().freeMemory() / 10, Integer.MAX_VALUE); |
There was a problem hiding this comment.
some magic numbers explanation might be handy here
| .createSession(getClient(), command); | ||
| Dialect dialect = result.getDialect(); | ||
| setCommandCodec(dialect.getCommandCodec()); | ||
| for (Map.Entry<String, CommandInfo> entry : getAdditionalCommands().entrySet()) { |
There was a problem hiding this comment.
can this be done with forEach?
|
@mykola-mokhnach will make required improvements soon. |
|
@mykola-mokhnach There are updates |
|
|
||
| return result.map(result1 -> { | ||
| Result toReturn = result.get(); | ||
| System.out.print(format("Detected dialect: %s", toReturn.getDialect())); |
There was a problem hiding this comment.
cannot we use logger object instead of System.print?
| .createSession(getClient(), command); | ||
| Dialect dialect = result.getDialect(); | ||
| setCommandCodec(dialect.getCommandCodec()); | ||
| getAdditionalCommands().forEach(this::defineCommand); |
| @Override | ||
| public Response execute(Command command) throws WebDriverException { | ||
| public Response execute(Command command) throws WebDriverException, IOException { |
There was a problem hiding this comment.
Is it a good idea to make it throwing the checked exception? Id rather wrap IOException into WebDriverException and keep the previous method signature
|
@mykola-mokhnach There new updates. Sorry for the delaying. I was busy. |
Change list
Types of changes
Details
It seems it is needed to propose some changes to Selenium. Make some methods protected, for instance.