Automated Tests to cover Stake Pool Settings and Wallet Currency Display Settings screens #2622

ManusMcCole · 2021-07-14T14:58:28Z

This PR adds automated tests for the Stake-pool server settings and also the Wallet currency display settings screens.

Testing Checklist

Slack QA thread
Test

Review Checklist

Basics

Code Quality

Important parts of the code are properly commented and documented
Code is properly typed with flow
React components are split-up enough to avoid unnecessary re-renderings
Any code that only works in main process is neatly separated from components

Testing

New feature/change is covered by acceptance tests
New feature/change is manually tested and approved by QA team
All existing acceptance tests are still up-to-date
New feature/change is covered by Daedalus Testing scenario
All existing Daedalus Testing scenarios are still up-to-date

After Review

Merge the PR
Delete the source branch
Move the ticket to done column on the YouTrack board
Update Slack QA thread by marking it with a green checkmark

…om knowing which function to run

danielmain

Looks pretty good! 😎

danielmain · 2021-07-15T07:19:58Z

tests/settings/e2e/steps/currency-display-settings.js

@@ -0,0 +1,43 @@
+// @flow
+import { When, Then} from 'cucumber';


Did you execute prettier in this file?

@ManusMcCole add a space after "Then"
e.g. import { When, Then } from 'cucumber';
or run yarn prettier:format

tests/settings/e2e/steps/currency-display-settings.js

danielmain · 2021-07-15T07:22:50Z

tests/settings/e2e/steps/currency-display-settings.js

+});
+
+Then(/^The wallet summary screen (does|does not) show ada balance in other currency's placeholder$/, function(switchstatus) {
+  let invisible = true;


You could write this in one line const invisible:boolean = switchstatus !== 'does'

The return of a comparison is always a boolean.

I like this solution very much. It condenses 4 lines of code in to 1 awesome 👍

danielmain · 2021-07-15T07:29:05Z

tests/settings/e2e/steps/stakepool-settings.js

+});
+
+Then(/^"([^"]*)" is visible on stake-pool screen above stake-pool list and is clickable$/, function(serverUrl) {
+  return this.waitAndClick('//*[@class="StakePools_smashSettings"]//span[text()="Moderated by '+ serverUrl + '"]');


If you want to write modern Javascript you could use Template_literals this instead string concatenation https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals 😉

I would look like this:

nikolaglumac · 2021-07-16T09:57:37Z

@daniloprates @tomislavhoracek please review this one 🙏

tomislavhoracek · 2021-07-16T10:16:17Z

tests/settings/e2e/features/currency-display-settings.feature

@@ -0,0 +1,33 @@
+@e2e @watch


@ManusMcCole Please remove the watch flag

nikolaglumac · 2021-07-16T10:17:34Z

tests/settings/e2e/features/currency-display-settings.feature

+    Then The wallet summary screen does not show ada balance in other currency's placeholder
+    When I am on the General Settings "wallets" screen
+    And I toggle the button on to change if I want to see my ada balance in other currency's
+    When I am on the General Settings "wallets" screen


@ManusMcCole I don't think we need multiple of these When I am on the General Settings "wallets" screen steps?

tomislavhoracek · 2021-07-16T10:19:27Z

tests/settings/e2e/features/currency-display-settings.feature

+    Then The wallet summary screen does not show ada balance in other currency's placeholder
+    When I am on the General Settings "wallets" screen
+    And I toggle the button on to change if I want to see my ada balance in other currency's
+    When I am on the General Settings "wallets" screen


@ManusMcCole you can remove this, you are already there

tomislavhoracek · 2021-07-16T10:21:53Z

tests/settings/e2e/features/stakepool-settings.feature

+    And I enter custom server "https://smash.cardano-testnet.iohkdev.io/" as the custom server option
+    And I click on Daedalus logo to change focus
+    And I click the stake-pool custom server input box submit button
+    And I see the your changes have been saved success message


You can declare a message like "your changes have been saved" and check if that is correct.
e.g. And I see the "Your changes have been saved" success message

tomislavhoracek · 2021-07-16T10:22:39Z

tests/settings/e2e/features/stakepool-settings.feature

+    And I select custom server option
+    Then The smash server input textBox is visible
+    And I enter invalid url "www.test" without https
+    Then Stake-pool custom input box error message is displayed


Maybe would be good to check if correct error message is shown

Indeed, you could have something like this

Then Stake-pool custom input box "https" error message is displayed

And in the JS you capture "https" and check if it's the correct error message. In other tests you could have something like

Then Stake-pool custom input box "invalidUrl" error message is displayed

Fixed now :)

tomislavhoracek · 2021-07-16T10:23:22Z

tests/settings/e2e/features/stakepool-settings.feature

+    Then Stake-pool custom input box error message is displayed
+    And I delete values in smash server custom url input box
+    And I enter invalid server "https://www.google.ie" containing https
+    And I click on Daedalus logo to change focus


Instead of declaring where you exactly clicked you can say, "I clicked outside something"

tomislavhoracek · 2021-07-16T10:25:43Z

tests/settings/e2e/steps/currency-display-settings.js

+const SELECT_CURRENCY_DROPDOWN = '//*[@label="Select currency"]';
+
+When(/^I open currency selection dropdown$/, function() {
+  return this.waitAndClick(SELECT_CURRENCY_DROPDOWN);


I think that you can use just click since you don't need to wait for something. Please try... :)

I must have not implemented this right ?

@ManusMcCole for native methods, it needs this.client. .... waitAndClick is a helper method created by us, therefore client is not needed.

…re/manus-automation

…file

ManusMcCole · 2021-09-03T08:39:20Z

Hi @input-output-hk/daedalus-dev. If somebody could review and possibly approve this PR soon it would be much appreciated. @nikolaglumac is keen to get it merged soon

thedanheller

Nice work @ManusMcCole. I've left some comments.

thedanheller · 2021-09-08T12:46:52Z

tests/settings/e2e/steps/stakepool-settings.js

@@ -0,0 +1,83 @@
+import { When, Then } from 'cucumber';
+import en from '../../../../source/renderer/app/i18n/locales/en-US.json';


It's important to give variables meaningful and easy-to-understand names. In this case, it could be something a bit less vague, like enLocales or enCopy

thedanheller · 2021-09-08T12:49:55Z

tests/settings/e2e/steps/stakepool-settings.js

+const STAKE_POOL_SERVER_DROPDOWN_CUSTOM_OPTION = `//*[@class="ScrollbarsCustom-Content"]//span[text()="${CUSTOM_SERVER_TEXT}"]`;
+const STAKE_POOLS_SUBMENU_SETTINGS = `//*[@class="SettingsMenu_component"]//button[text()="${STAKE_POOL_TEXT}"]`;
+
+When(/^custom server is the default option$/, function() {


The Custom Server option should not be the default one. It should be IOHK (Recommended). If Custom Server is appearing by default there might be a problem with environment variables.

Hi @daniloprates. Thank you for the review. On my side in selfnode all i can see are options for custom and "None fetch the data directly screenshot I had presumed that it was the same for everybody. What environment variables do i need to update ?

Where are these environment variables set ? @daniloprates

Manus the variable can be found at daedalus.stores.staking.smashServerUrl.

For testnet, the default server url is https://smash.cardano-testnet.iohkdev.io. For mainnet, it's something like https://smash.cardano.iohk.io, but I didn't have time to check, because mainnet was too outdated in my computer.

You can indeed use this URL for testing a custom server, but you need to check the network, because if it's testnet, it will lead to a different behaviour in the app, as it's the default server, not a custom one. In this case, you need to use another URL, like the mainnet one.

thedanheller · 2021-09-08T13:30:58Z

tests/settings/e2e/features/stakepool-settings.feature

+
+  Scenario: Navigating to Stake-pool settings screen and selecting custom stake-pool server
+    Given I am on the General Settings "stake-pools" screen
+    When custom server is the default option


This step is not very clear. In case another option is selected, is it supposed to select the Custom option? If so, it should instead click the dropdown, then click the Custom option, rather than select the input element

nikolaglumac · 2021-09-13T06:41:37Z

@ManusMcCole please try to get this one done today 🙏

nikolaglumac · 2021-09-13T07:07:45Z

FYI. @ManusMcCole I have merged in latest develop branch so please pull before committing new changes 🙏

…re/manus-automation

Fix Merge Conflict

…put-hk/daedalus into chore/manus-automation

ManusMcCole added 9 commits July 14, 2021 11:42

Added tests for Stake-pool Settings and Wallet Currency Display Settings

bd3a212

Deleted old comments and unneeded timeout

57bee26

Fixing typo in stakepool-settings.feature that was preventing code fr…

8d51456

…om knowing which function to run

Minor Code reorganising

7e43bdd

Merge branch 'develop' into chore/manus-automation

27f3f13

Minor Code reorganisation

45593b2

Correcting lint spelling errors

7eb5338

Added entry to CHANGELOG.md

7788106

Adding an extra feature step to improve code reliability

f822ed5

ManusMcCole changed the title ~~Chore/manus automation~~ Automated Tests to cover Stake Pool settings Screen and Wallet Currency Display settings Jul 14, 2021

ManusMcCole changed the title ~~Automated Tests to cover Stake Pool settings Screen and Wallet Currency Display settings~~ Automated Tests to cover Stake Pool settings and Wallet Currency Display settings screens Jul 14, 2021

ManusMcCole changed the title ~~Automated Tests to cover Stake Pool settings and Wallet Currency Display settings screens~~ Automated Tests to cover Stake Pool Settings and Wallet Currency Display Settings screens Jul 14, 2021

ManusMcCole requested review from nikolaglumac and a team July 14, 2021 15:05

Added link to CHANGELOG.md

f26c24d

danielmain reviewed Jul 15, 2021

View reviewed changes

nikolaglumac assigned ManusMcCole Jul 15, 2021

nikolaglumac added ⏳release-vNext chore labels Jul 15, 2021

nikolaglumac requested review from tomislavhoracek and thedanheller July 16, 2021 09:57

tomislavhoracek reviewed Jul 16, 2021

View reviewed changes

nikolaglumac reviewed Jul 16, 2021

View reviewed changes

tomislavhoracek reviewed Jul 16, 2021

View reviewed changes

nikolaglumac and others added 8 commits August 23, 2021 10:54

Reorder CHANGELOG

6284b6c

Importing Daedalus text and labels from file en-US.json

e1b67fb

Merge branch 'develop' into chore/manus-automation

3d6c606

Merge remote-tracking branch 'origin/chore/manus-automation' into cho…

1dcbe14

…re/manus-automation

Added 'Your changes have been saved' success message from en-US.json …

37d678b

…file

Removed @watch flags

da08491

Merge branch 'develop' into chore/manus-automation

5091d6a

Readding entry to CHANGELOG.md

671f211

ManusMcCole requested a review from thedanheller August 31, 2021 08:21

Merge branch 'develop' into chore/manus-automation

bb44af7

thedanheller suggested changes Sep 8, 2021

View reviewed changes

Merges develop

b97a867

ManusMcCole and others added 15 commits September 13, 2021 15:15

Updating constant name for linking to en-US.json' file

fb3d023

Merge branch 'develop' into chore/manus-automation

56a4dc8

Merge branch 'develop' into chore/manus-automation

9132303

Upgraded cucumber and spectron

ff47a9e

Fix wrong path for starting electron

b3d951c

Code refactor.Adding condition for testnet

a54f6e3

Merge remote-tracking branch 'origin/chore/manus-automation' into cho…

a20c465

…re/manus-automation

Minor formatting

9c723b9

Starting Spectron like the official documentation suggests

f5aee02

Merge branch 'develop' into chore/manus-automation

b5dced4

Merges develop

88970fb

Merge branch 'develop' into chore/manus-automation

a4335f2

Fix Merge Conflict

Merge branch 'chore/manus-automation' of https://github.com/input-out…

be36f43

…put-hk/daedalus into chore/manus-automation

Merge branch 'develop' into chore/manus-automation

f668343

Renamed all js files to ts

fdcff17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automated Tests to cover Stake Pool Settings and Wallet Currency Display Settings screens #2622

Automated Tests to cover Stake Pool Settings and Wallet Currency Display Settings screens #2622

ManusMcCole commented Jul 14, 2021 •

edited by nikolaglumac

Loading

danielmain left a comment

danielmain Jul 15, 2021

tomislavhoracek Jul 16, 2021

danielmain Jul 15, 2021

ManusMcCole Jul 16, 2021

danielmain Jul 15, 2021

nikolaglumac commented Jul 16, 2021

tomislavhoracek Jul 16, 2021

nikolaglumac Jul 16, 2021

tomislavhoracek Jul 16, 2021

tomislavhoracek Jul 16, 2021

tomislavhoracek Jul 16, 2021

thedanheller Jul 16, 2021

ManusMcCole Jul 22, 2021

tomislavhoracek Jul 16, 2021

tomislavhoracek Jul 16, 2021

ManusMcCole Jul 19, 2021

thedanheller Aug 3, 2021

ManusMcCole commented Sep 3, 2021

thedanheller left a comment

thedanheller Sep 8, 2021

ManusMcCole Sep 13, 2021

thedanheller Sep 8, 2021

ManusMcCole Sep 13, 2021

ManusMcCole Sep 17, 2021

thedanheller Sep 17, 2021

thedanheller Sep 8, 2021

nikolaglumac commented Sep 13, 2021

nikolaglumac commented Sep 13, 2021

		@@ -0,0 +1,43 @@
		// @flow
		import { When, Then} from 'cucumber';

		@@ -0,0 +1,83 @@
		import { When, Then } from 'cucumber';
		import en from '../../../../source/renderer/app/i18n/locales/en-US.json';

Automated Tests to cover Stake Pool Settings and Wallet Currency Display Settings screens #2622

Are you sure you want to change the base?

Automated Tests to cover Stake Pool Settings and Wallet Currency Display Settings screens #2622

Conversation

ManusMcCole commented Jul 14, 2021 • edited by nikolaglumac Loading

Testing Checklist

Review Checklist

Basics

Code Quality

Testing

After Review

danielmain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikolaglumac commented Jul 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManusMcCole commented Sep 3, 2021

thedanheller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikolaglumac commented Sep 13, 2021

nikolaglumac commented Sep 13, 2021

ManusMcCole commented Jul 14, 2021 •

edited by nikolaglumac

Loading