Skip to content
This repository has been archived by the owner on Nov 23, 2022. It is now read-only.

started work on E2E Tests #298

Merged
merged 32 commits into from Jul 16, 2020
Merged

started work on E2E Tests #298

merged 32 commits into from Jul 16, 2020

Conversation

DerMolly
Copy link
Member

Component/Part

E2E Tests

Description

This PR adds E2E Tests.

Steps

  • added implementation
  • added / updated tests
  • added / updated documentation
  • extended changelog

Related Issue(s)

#11

@DerMolly DerMolly self-assigned this Jun 27, 2020
@DerMolly
Copy link
Member Author

somehow the firefox e2e test errors and I'm currently a bit clueless why

@DerMolly
Copy link
Member Author

now there seems to be a problem with typescript in the CI

a tutorial how to use typescript with cypress (maybe we need a webpack config) https://glebbahmutov.com/blog/use-typescript-with-cypress/

@DerMolly DerMolly marked this pull request as ready for review June 30, 2020 18:52
@DerMolly DerMolly added the type: code quality enhancement An improvement to quality of code or config files label Jun 30, 2020
README.md Outdated Show resolved Hide resolved
cypress.json Show resolved Hide resolved
Comment on lines +26 to +32
"privacy": "https://example.com/privacy",
"termsOfUse": "https://example.com/termsOfUse",
"imprint": "https://example.com/imprint"
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be removed again, but then again it's not really necessary since it's mock data anyway

Copy link
Member

@mrdrogdrog mrdrogdrog left a comment

Choose a reason for hiding this comment

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

I don't think that you need most of the ids. Especially because I can use the same id multiple times (even if I shouldn't). E.g. For the external links: why don't you get all a tags in the footer and check how many with the wanted urls are present?

pull_request:
branches: [master]

jobs:
Copy link
Member

Choose a reason for hiding this comment

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

It seems like both jobs are the same, but the browser. How about the matrix strategy, like we used in the build job?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can I use the matrix parameters just in with statements?

Or should this also work?

end2end:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        browser: ['e2e:chrome', 'e2e:firefox']
    name: e2e Test Chrome
    steps:
      ...
      - name: Run cypress tests
        run: yarn ${{ matrix.browser }}

Copy link
Member

@mrdrogdrog mrdrogdrog Jul 4, 2020

Choose a reason for hiding this comment

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

you can use it everywhere, where you want to insert a value. (please remember changing the job names, too)

Copy link
Member Author

Choose a reason for hiding this comment

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

done

expect(localStorage.getItem('bannerTimeStamp')).to.be.null
})

it('text correct', () => {
Copy link
Member

Choose a reason for hiding this comment

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

If cypress handles the test cases like other frameworks, then the content should be written like if you want to complete a sentence. In this case it would be "shows the correct alert banner text"

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed some of the descriptions


it('text dismissable', () => {
cy.get('.alert-primary.show').contains(banner.text)
cy.get('.alert-primary.show').find('.fa-times').click().then(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Please use multiple lines, if you have builder-pattern-like chains

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed that

@@ -15,15 +15,15 @@ export const PoweredByLinks: React.FC = () => {
return (
<p>
<Trans i18nKey="landing.footer.poweredBy">
<ExternalLink href="https://codimd.org" text="CodiMD"/>
<ExternalLink href="https://codimd.org" text="CodiMD" id='codimd'/>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a good id, because it's too general. Why do you need an id? Can't you do a query for a link that points to this url? Like a[href=https://CodiMD.org] . See css attribute selector.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed that

@DerMolly DerMolly requested a review from mrdrogdrog July 4, 2020 23:04
@DerMolly DerMolly added this to Review in progress in Release 2.0 Jul 5, 2020
@DerMolly DerMolly moved this from Review in progress to In progress in Release 2.0 Jul 5, 2020
@ErikMichelson ErikMichelson linked an issue Jul 5, 2020 that may be closed by this pull request
Release 2.0 automation moved this from In progress to Review in progress Jul 6, 2020
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cypress/.eslintrc.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@DerMolly DerMolly removed the request for review from mrdrogdrog July 16, 2020 09:18
@DerMolly DerMolly merged commit f0fe7f5 into master Jul 16, 2020
Release 2.0 automation moved this from Review in progress to Done Jul 16, 2020
@DerMolly DerMolly deleted the feature/e2eStart branch July 16, 2020 09:22
@SISheogorath SISheogorath removed this from Done in Release 2.0 Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: code quality enhancement An improvement to quality of code or config files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add browser e2e tests
4 participants