It takes some time for the viewer alert to be updated after the editor
is committed, but the current tests don't await that and proceed too
fast to the viewer alert string assertion. This commit fixes the issue
by waiting for the expected viewer alert string to appear instead.
The `Page.evaluate()` and `Mouse.click()` APIs in Puppeteer both return
a promise; see https://pptr.dev/api/puppeteer.page.evaluate and
https://pptr.dev/api/puppeteer.mouse.click, and should therefore be
awaited before proceeding in tests to make sure that the test's behavior
is deterministic and avoid intermittent failures.
The following command was used to find potential places to fix:
`grep -nEr "[^await|return] page\." test/integration/*`
This integration test fails intermittently because the undo button can
only be activated if focus can be put on it, and that in turn can only
happen if it's visible. The test tried to make sure that the undo bar
is visible, but checking for the absence of the `hidden` attribute is
unfortunately not enough to assert visibility according to Puppeteer
documentation [1]. Moreover, the undo button wasn't checked at all.
To fix the issue we let Puppeteer do the visibility detection for the
undo bar by providing the `visible: true` option to `waitForSelector`
[2]. This is consistent with the other tests that already do this, and
also with the existing code that detects if the undo bar is hidden
(which uses the `hidden: true` option of `waitForSelector`). Moreover,
we wait for the undo button to be present before putting focus on it.
For consistency, and to avoid intermittent failures elsewhere, we mirror
this solution to the other undo bar/button tests of the various editors.
[1] https://pptr.dev/api/puppeteer.elementhandle.isvisible
[2] https://pptr.dev/api/puppeteer.waitforselectoroptions
This replaces the various copies of this logic with a single helper that
we template for each editor type, similar to what we already do for the
`switchToEditor` helper.
This replaces the various copies of this logic with a single helper that
we template for each editor type, similar to what we already do for the
`switchToEditor` helper.
When a drawing was moved with arrow keys and then printed or saved, the drawing wasn't moved finally.
So the fix is just about calling onTranslated once the translation is done.
To avoid being able to introduce dependencies between tests, and to
bring existing dependencies to the surface, this commit makes sure that
we close the document between tests so that we can't accidentally rely
on state set by a previous test. This prevents multiple tests from
failing if one of them fails and makes debugging easier by being able to
run each test on their own independent of other tests.
This commit, combined with the previous one, is enough to make the ink
editor integration test suite pass consistently if random mode in
Jasmine is enabled, proving that the tests are fully isolated now.
The second test of the basic operations block for the ink editor
depends on the first test to work. This becomes visible if we only run
the second test, using `fit`, which always fails with:
`ProtocolError: Waiting for selector '.annotationEditorLayer' failed:
Runtime.callFunctionOn timed out. Increase the 'protocolTimeout' setting
in launch/connect calls for a higher timeout if needed.`
The problem is that the second test doesn't enable the ink editor and
relies on the first test having done that already (because we don't
close the document between tests yet). This commit fixes the issue by
unconditionally enabling the ink editor in the second test to remove the
dependency between the two tests so they both pass in isolation.
This patch fixes a bug that caused incorrect curve shapes when an endpoint lies beyond the page boundaries. It adds a check for the endpoint's position, and if it is outside the page, the point is excluded from the shape's coordinates.
In most integration tests we already use the pattern of defining the
editor selector once and reusing it in the rest of the test, but it's
not fully consistent everywhere yet. This commit fixes that for the
ink editor integration tests, which has multiple advantages:
- it improves consistency between the various editor integration tests;
- it removes duplicate function calls and aligns variable definitions;
- it reduces the number of `getEditorSelector` calls that contained
hardcoded IDs, which helps to isolate the tests and to simplify
follow-up patches.
This has multiple advantages:
- it improves consistency between the various editor integration tests;
- it makes the code easier to read/understand;
- it reduces code duplication;
- it reduces the number of `getEditorSelector` calls that contained
hardcoded IDs, which helps to isolate the tests and to simplify
follow-up patches.
The `getSelectedEditors` function is largely a copy of the `getEditors`
function, with three small differences:
1. `getEditors` allows getting any kind of editor whereas
`getSelectedEditors` is harcoded to only getting selected editors.
2. `getEditors` returns editor selectors (strings) whereas
`getSelectedEditors` returns editor IDs (integers).
3. `getSelectedEditors` returns a sorted array of editor IDs whereas
`getEditors` does not ensure that the array is sorted.
This commit makes the `getEditors` function a drop-in replacement for
the `getSelectedEditors` function to deduplicate the code and to have a
unified way of getting editors.
Note that we don't actually use the contents of the returned array
(only its length), so we can safely change `getEditors` to return a
sorted array of integer editor IDs instead. Sorting the array makes the
return value deterministic, which is a nice property for test stability,
and integer IDs are also easier to handle in test assertions. Note that
the corresponding selector strings can also easily be obtained from the
integer IDs using the `getEditorSelector` function if needed.
It was due the resize observer which is removed thanks to this patch.
In order to reuse the dragAndDrop function in test, this patch slighty refactors it
in order to make it easier to use.
This patch makes a clear separation between the way to draw and the editing stuff.
It adds a class DrawEditor which should be extended in order to create new drawing tools.
As an example, the ink tool has been rewritten in order to use it.
Over time the number of integration tests that get the rectangle for a
given selector has increased quite a bit, and the code to do so has
consequently become duplicated.
This commit refactors the integration tests to move the rectangle
fetching code to a single place, which reduces the code by over 400
lines and makes the individual tests simpler.
The original bug was because the parent was null when trying to show
an highlight annotation which led to an exception.
That led me to think about having some null/non-null parent when removing
an editor: it's a mess especially if a destroyed parent is still attached
to an editor. Consequently, this patch always sets the parent to null when
deleting the editor.
