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

Introduce yarn run test:full #51

Merged
merged 6 commits into from
Jan 14, 2020
Merged

Introduce yarn run test:full #51

merged 6 commits into from
Jan 14, 2020

Conversation

florian-richter
Copy link
Contributor

Proposed Changes

There is a small number of very slow tests. This makes it annoying to run yarn test locally. Now I introduced yarn run test:full and excluded the slow tests when using yarn test.

Checklist

  • I have added or adjusted tests that prove my fix is effective or that my feature works

Copy link
Contributor

@mr-flannery mr-flannery left a comment

Choose a reason for hiding this comment

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

I think generally an approach like this is fine, but I'm afraid that we're opening Pandora's box here, speaking from a cultural perspective. So I do want to raise the questions: why are these tests slow? And is there something we can do about it?

@florian-richter
Copy link
Contributor Author

They are not actually unit tests, but end-to-end tests since they are installing dependencies via npm or creating a new project via the nest cli (through npx).

Since they are fairly contained from the code that is being checked, I somewhat like that they are in the same file as related unit tests. I guess it isn't a best practice to mix different levels of tests like this, but the alternatives seem worse in our specific case (splitting the files, using different test frameworks, ...)

@mr-flannery
Copy link
Contributor

It's fine for me for now. I think it would be good to get feedback from at least one other person (@marikaner @FrankEssenberger @jjtang1985).

@marikaner
Copy link
Contributor

I agree with @mr-flannery here, but I see that this is probably the most pragmatic solution. Still, I am not a fan of the framing of "SLOW" tests as it has a negative connotation in terms of tests. Consider whether "E2E" or "INTEGRATION" are better names.

@florian-richter florian-richter merged commit b8a8765 into master Jan 14, 2020
@florian-richter florian-richter deleted the update-dependencies branch January 14, 2020 08:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants