Skip to content

refactor: tests#1986

Merged
alexander-akait merged 1 commit into
masterfrom
refactor-tests
Jun 6, 2019
Merged

refactor: tests#1986
alexander-akait merged 1 commit into
masterfrom
refactor-tests

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Jun 6, 2019

Copy link
Copy Markdown
Member
  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

not required

Motivation / Use-Case

Refactor test part #1

Breaking Changes

No

Additional Info

Ideally each option should have:

  1. Unit test (own)
  2. Integration test (Server.test.js)
  3. E2E test (can be avoided for non client options)
  4. CLI test (can be avoided right now)

We should concentrate on 1 and 2 points right now.

Other changes what we should do:

  • avoid using --runInBand it is decrease time on tests
  • union option.test.js and validation.test.js (they do same)
  • rewrite some test in more effective way
  • better naming of tests

@alexander-akait alexander-akait requested a review from hiroppy as a code owner June 6, 2019 18:26
@alexander-akait alexander-akait mentioned this pull request Jun 6, 2019
Merged
6 tasks
@hiroppy

hiroppy commented Jun 6, 2019

Copy link
Copy Markdown
Contributor

maybe we can not avoid using --runInBand because many tests have same port.

@alexander-akait

alexander-akait commented Jun 6, 2019

Copy link
Copy Markdown
Member Author

@hiroppy We can setup for tests using difference port 😄

@hiroppy

hiroppy commented Jun 6, 2019

Copy link
Copy Markdown
Contributor

Yes, but we should consider managing ports because it is easy to conflict ports.

@alexander-akait

alexander-akait commented Jun 6, 2019

Copy link
Copy Markdown
Member Author

I think we can use getPort utils in test helper, it is require refactor, but it is possible and tests always use difference port

hiroppy
hiroppy previously approved these changes Jun 6, 2019
View reviewed changes

@hiroppy hiroppy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM if ci is green 💯

@alexander-akait

alexander-akait commented Jun 6, 2019

Copy link
Copy Markdown
Member Author

Anyway let's do it in other PRs, not here 😄

@alexander-akait

alexander-akait commented Jun 6, 2019

Copy link
Copy Markdown
Member Author

Missing snapshot (updated)

@alexander-akait alexander-akait merged commit 638d7a1 into master Jun 6, 2019
@alexander-akait alexander-akait deleted the refactor-tests branch June 6, 2019 18:58
hiroppy pushed a commit that referenced this pull request Jun 6, 2019
@evilebottnawi @hiroppy
refactor: tests (#1986)
918f88b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@hiroppy hiroppy hiroppy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alexander-akait @hiroppy