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

Add frontend test suite #1

Closed
wants to merge 0 commits into from
Closed

Add frontend test suite #1

wants to merge 0 commits into from

Conversation

fersaru
Copy link
Collaborator

@fersaru fersaru commented Jan 18, 2024

Adds the WebdriverIO test suite.

  • The content is according to the continuous integration guide of tauri with minimal changes to the github workflow yml .github/workflows/webdriver.yml.
  • The defined tests in webdriver/webdriverio/test/specs/example.e2e.js are not adjusted to the current frontend but serve as an example.

Copy link
Owner

@falk-werner falk-werner left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I have two remarks on it:

  1. Please make sure that your PR works before requesting. You may use a draft PR for unfinished works.
  2. I'm not sure if I want to add webdriver-based tests in the current project state

While webdriver is useful for larger project to test UI, it also requires some considerations of how we structure our UI, since we need a proper way to find elements in DOM an acting with them. This may be okay, since one goal of this project is to learn more about technology. Maybe we need some more UI before activating webdriver. Maybe we should add some unit tests first.

@@ -0,0 +1,77 @@
# run this action when the repository is pushed to
Copy link
Owner

Choose a reason for hiding this comment

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

Please make the workflow run successfully. In it's the current state, it fails.

Comment on lines 8 to 10
"dependencies": {
"@wdio/cli": "^7.34.0"
},
Copy link
Owner

Choose a reason for hiding this comment

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

This should be a test dependency. It is not needed during runtime. Please remove and re-add it using npm install --test.

@@ -0,0 +1,32 @@
// calculates the luma from a hex color `#abcdef`
Copy link
Owner

Choose a reason for hiding this comment

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

This spec seems to be very generic and it will fail for our application. Please replace it with a useful first test for note.rs, e.g. you may test for the presence of title bar buttons are present or the transition from main to settings screen.

@@ -0,0 +1,293 @@
exports.config = {
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove unnecessary boilerplate (most of the comments).

@fersaru fersaru closed this Jan 19, 2024
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.

2 participants