Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add more E2E test suites for Detox Copilot #4623

Merged
merged 8 commits into from
Nov 4, 2024

Conversation

LironMShemen
Copy link
Collaborator

Added new E2E test suites for Detox Copilot, made several improvements to the Copilot driver.
Resolves #4612

@LironMShemen LironMShemen self-assigned this Oct 28, 2024
@LironMShemen LironMShemen requested review from asafkorem and removed request for noomorph and d4vidi October 28, 2024 18:10
Copy link
Contributor

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

Good stuff! 👏🏼

detox/test/e2e/copilot/07.copilot.assertions.test.js Outdated Show resolved Hide resolved
Comment on lines 22 to 33
it('should assert an element is visible', async () => {
await copilot.perform(
'Find an element with ID "main-text" in the Assertions',
'Verify that the text of this element is "i contain some text"',
);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to assert this in one step (Verify there's an element with the text "..."..)

Copy link
Contributor

Choose a reason for hiding this comment

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

we need to have such test for each kind of assertion (there's an element with the text / id / label / type [e.g. "check-box"])

Copy link
Contributor

Choose a reason for hiding this comment

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

+ test that expects failures for invalid assertions (rejects.toThrowError()) for each of the matching types

Copy link
Contributor

Choose a reason for hiding this comment

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

do you have more ideas for assertions?
we're not limited to the "old" types of matchers, let's include more types of assertions that is reasonable to perform with Copilot

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, there are tests for the following:

  • Assertion with id + expects failures for invalid value
  • Assertion with text + expects failures for invalid value
  • Assertion by label + expects failures for invalid value
  • Assertion by type + expects failures for invalid value

More options that may be relevant:

  • Assert that elements are in a specific order
  • Assert that an animation occurs within a specific timeframe
  • Assert that an image loads correctly and has the expected source
  • Assert that an element is in a specific color

Copy link
Contributor

Choose a reason for hiding this comment

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

Assert that elements are in a specific order - 👍🏼
Assert that an animation occurs within a specific timeframe - 👎🏼
Assert that an image loads correctly 👍🏼 and has the expected source 👎🏼
Assert that an element is in a specific color 👍🏼


Perhaps assertion on element position? (e.g. bottom -right)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The assertions were added. They exposed a problem of the copilot to understand the order and locations of elements, mostly when expecting a test to fail. An issue was added.

detox/test/e2e/copilot/07.copilot.assertions.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/08.copilot.location.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/09.copilot.datepicker.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/09.copilot.datepicker.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/09.copilot.datepicker.test.js Outdated Show resolved Hide resolved
detox/test/src/Screens/DatePickerScreen.js Show resolved Hide resolved

it('should not assert an element that does not exist (by label)', async () => {
await jestExpect(async () =>
await copilot.perform('Find an element with label "Does not exist" in the Assertions')
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not exist is too explicit for the copilot, try something with "Banana Split" or "I contain lots of text" that resembles a text that actually exists

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to: "I am full of text" and "I exist in the screen"

detox/test/e2e/copilot/07.copilot.assertions.test.js Outdated Show resolved Hide resolved
Comment on lines 60 to 62
it('shouldn`t assert an element that does not exist (by type)', async () => {
await jestExpect(async () =>
await copilot.perform('Find a Check-box type element in the Assertions')
Copy link
Contributor

Choose a reason for hiding this comment

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

add a test with another element type: perhaps text-field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With text-field the test passed.
The checkbox test is skipped for now with a note that describes the doubts we have.

detox/test/e2e/copilot/07.copilot.assertions.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/08.copilot.location.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/08.copilot.location.test.js Outdated Show resolved Hide resolved
detox/test/src/Screens/DatePickerScreen.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

detox/test/e2e/copilot/07.copilot.assertions.test.js Outdated Show resolved Hide resolved
detox/test/e2e/copilot/07.copilot.assertions.test.js Outdated Show resolved Hide resolved
Comment on lines +2 to +5
const DUMMY_COORDINATE1_LONGITUDE = '66.5';
const DUMMY_COORDINATE1_LATITUDE = '-80.125';
const DUMMY_COORDINATE2_LONGITUDE = '-80.125';
const DUMMY_COORDINATE2_LATITUDE = '66.5';
Copy link
Contributor

Choose a reason for hiding this comment

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

perfect

Including assertions screen changes
test: Change the launching app date to constant date

Instead of generating the current date and time. For more stable testing, and for the copilot cache.

including changes in the date picker screen
The cache file was modified as a result of incorporating four new test suites: assertions, location, datepicker and visibility.
@LironMShemen LironMShemen force-pushed the E2E-test-suites-for-detox-copilot branch from 02b1e55 to 6834970 Compare November 4, 2024 14:09
@asafkorem asafkorem enabled auto-merge November 4, 2024 14:12
@asafkorem asafkorem merged commit 0c652db into master Nov 4, 2024
3 checks passed
@asafkorem asafkorem deleted the E2E-test-suites-for-detox-copilot branch November 4, 2024 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more E2E test suites for Detox Copilot
2 participants