On Wed, 23 Nov 2022 23:38:59 GMT, John Hendrikx wrote:
>> Yes, but this is now getting to be a bit out of scope for this PR. Even
>> adding the `assertTrue` requires running all the tests (which thankfully,
>> GHA will do).
>>
>> Perhaps a better approach is to not make any more changes here,
On Wed, 23 Nov 2022 23:27:21 GMT, Kevin Rushforth wrote:
>> could we just create a method in Util instead of dragging multi-line
>> construct across the tests?
>>
>> e.g. `Util.loadStubToolkit()` ?
>
> Yes, but this is now getting to be a bit out of scope for this PR. Even
> adding the `assert
On Wed, 23 Nov 2022 23:21:52 GMT, Andy Goryachev wrote:
>> I wouldn't recommend that, since it will then obscure the main reason for
>> this call -- to load the toolkit (honestly, the cast was probably an
>> afterthought). So this would be OK:
>>
>>
>> tk = Toolkit.getToolkit();//This
On Wed, 23 Nov 2022 23:14:52 GMT, Kevin Rushforth wrote:
>> I can replace these with an assert:
>>
>> assertTrue(Toolkit.getToolkit() instanceof StubToolkit);
>
> I wouldn't recommend that, since it will then obscure the main reason for
> this call -- to load the toolkit (honestly, the cast
On Wed, 23 Nov 2022 23:09:55 GMT, John Hendrikx wrote:
>>> As for this PR, I don't mind leaving it as is or reverting these changes. I
>>> don't think that a CCE is realistic here.
>>
>> I agree that this isn't a good test design. I was just pointing out why it
>> isn't (in theory) a completel
On Wed, 23 Nov 2022 22:57:07 GMT, Kevin Rushforth wrote:
>> This looks like bad test design. If the toolkit needs to be of a specific
>> type when running tests, then check that this is the case once before
>> running all the tests. As for this PR, I don't mind leaving it as is or
>> reverting
On Wed, 23 Nov 2022 22:00:16 GMT, Nir Lisker wrote:
>> Yes, good catch, fixed.
>
> Eclipse's quick fix is to replace the `instanceof` check with a `null` check,
> did it not suggest the same for you?
I was to quick on the trigger and removed it myself, not Eclipse fault.
-
PR: htt
On Wed, 23 Nov 2022 22:50:17 GMT, Nir Lisker wrote:
> As for this PR, I don't mind leaving it as is or reverting these changes. I
> don't think that a CCE is realistic here.
I agree that this isn't a good test design. I was just pointing out why it
isn't (in theory) a completely compatible cha
On Tue, 22 Nov 2022 19:04:00 GMT, Kevin Rushforth wrote:
>> Not needed in the setup, if it is `null` later or throws an exception the
>> test failure is clear enough.
>>
>> Also a bit out of scope of this PR.
>
> Yes, adding an assert does seem out of scope of this PR, although Andy
> comment
On Tue, 22 Nov 2022 18:48:41 GMT, John Hendrikx wrote:
>> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/FXVKSkin.java
>> line 858:
>>
>>> 856: protected void sendKeyEvents() {
>>> 857: Node target = fxvk.getAttachedNode();
>>> 858:
>>
>> same comm
On Tue, 22 Nov 2022 18:41:16 GMT, John Hendrikx wrote:
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/AccordionTest.java
>> line 57:
>>
>>> 55:
>>> 56: @Before public void setup() {
>>> 57: tk = Toolkit.getToolkit();//This step is not needed (Just to
>>> make s
On Tue, 22 Nov 2022 17:38:37 GMT, Andy Goryachev wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert "Fix warnings in fxml"
>>
>> This reverts commit b148aa3cc8a4676167a9eb8a023cddce3de116e7.
>
> modules/j
On Tue, 22 Nov 2022 18:08:19 GMT, Andy Goryachev wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert "Fix warnings in fxml"
>>
>> This reverts commit b148aa3cc8a4676167a9eb8a023cddce3de116e7.
>
> modules/j
On Tue, 22 Nov 2022 16:44:01 GMT, John Hendrikx wrote:
>> Note: I ran into a `javac` compiler bug while replacing types with diamond
>> operators (ecj has no issues). I had two options, add a
>> `SuppressWarnings("unused")` or to use a lambda instead of a method
>> reference to make `javac` h
> Note: I ran into a `javac` compiler bug while replacing types with diamond
> operators (ecj has no issues). I had two options, add a
> `SuppressWarnings("unused")` or to use a lambda instead of a method reference
> to make `javac` happy. I choose the later and added a comment so it can be
>
15 matches
Mail list logo
