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

Fix ui test of ci #752

Merged
merged 21 commits into from
Sep 17, 2020
10 changes: 7 additions & 3 deletions .github/workflows/e2e-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,27 @@ jobs:
run: |
curl --proto '=https' --tlsv1.2 -sSf https://tiup-mirrors.pingcap.com/install.sh | sh
source /home/runner/.profile
tiup update --nightly
tiup playground nightly --tiflash=0 &
tiup update playground
source /home/runner/.profile
tiup playground v4.0.6 --tiflash=0 &
- name: Build UI
run: |
make ui
env:
NO_MINIMIZE: true
CI: true
REACT_APP_MIXPANEL_TOKEN: ""
- name: Wait TiUP Playground
run: |
chmod u+x scripts/wait_tiup_playground.sh
scripts/wait_tiup_playground.sh 15 20
- name: Debug TiUP
run: |
source /home/runner/.profile
tiup --version
ls /home/runner/.tiup/components/playground/
DATA_PATH=$(ls /home/runner/.tiup/data/)
echo $DATA_PATH
tiup playground display
echo "==== TiDB Log ===="
head -n 3 /home/runner/.tiup/data/$DATA_PATH/tidb-0/tidb.log
echo "==== TiKV Log ===="
Expand Down
22 changes: 22 additions & 0 deletions scripts/wait_tiup_playground.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash
# Wait unitl `tiup playground` command runs success

INTERVAL=$1
MAX_TIMES=$2

if ([ -z "${INTERVAL}" ] || [ -z "${MAX_TIMES}" ]); then
echo "Usage: command <interval> <max_times>"
exit 1
fi

source /home/runner/.profile

for ((i=0; i<${MAX_TIMES}; i++)); do
tiup playground display
if [ $? -eq 0 ]; then
exit 0
fi
sleep ${INTERVAL}
done

exit 1
1 change: 1 addition & 0 deletions ui/dashboardApp/layout/signin/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ function TiDBSignInForm({ successRoute, onClickAlternative }) {
<Input onInput={clearErrorMsg} prefix={<UserOutlined />} disabled />
</Form.Item>
<Form.Item
data-e2e="password"
name="password"
label={t('signin.form.password')}
{...(errorMsg && {
Expand Down
8 changes: 7 additions & 1 deletion ui/tests/e2e/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ describe('Login', () => {

await ppExpect(page).toFill('input#tidb_signin_password', 'any')
await ppExpect(page).toClick('button#signin_btn')
await ppExpect(page).toMatch('TiDB authentication failed')

const failReason = await page.waitForSelector(
'form#tidb_signin div[data-e2e="password"] div:last-child'
Copy link
Member

Choose a reason for hiding this comment

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

This div:last-child is too magical.. Can we await for pages to contain some string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems no way to add the extra flag to the error message self except to its parent container likes data-e2e="password".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or we can simply remove the div:last-child.

Copy link
Collaborator Author

@baurine baurine Sep 17, 2020

Choose a reason for hiding this comment

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

Just got what you mean. I print the failed reason content to help debug what the exact failed reason. It may not be "TiDB authentication failed", it may "Failed to connect to TiDB" etc. So when the UI test fails later, I can know what makes it fail.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, got it!

)
const content = await failReason.evaluate((n) => n.innerText)
console.log('fail reason:', content)
expect(content).toContain('TiDB authentication failed')
},
10 * 1000
)
Expand Down