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

change tests to not always pass #38

Merged
merged 10 commits into from
Dec 14, 2017
Merged

change tests to not always pass #38

merged 10 commits into from
Dec 14, 2017

Conversation

dmt
Copy link
Contributor

@dmt dmt commented Aug 9, 2017

#37

As mentioned in the issue, I don't really know what I'm doing but this way I can get tests to fail if I change expectations/expected behaviour.

@Strnadj
Copy link

Strnadj commented Aug 15, 2017

Please @dmt could you explain why you're using URI other than express app instance? According to supertest documentation you can request(app). I am just curious.

@dmt
Copy link
Contributor Author

dmt commented Aug 15, 2017

@Strnadj I didn't write those tests, I only changed the port.

I guess that if server.ts wasn't binding to the the port, you could just require it and use it for supertest but I didn't want to rewrite an app I really don't know anything about.

this seems a bit crude but allows running the tests and having them take
care of setting up the port to use
@markmssd
Copy link

markmssd commented Sep 2, 2017

+1, I think this is a very important PR that should be merged :/

@microsoft microsoft deleted a comment from msftclas Sep 26, 2017
@microsoft microsoft deleted a comment from msftclas Sep 26, 2017
@dmt
Copy link
Contributor Author

dmt commented Nov 23, 2017

Whatever happened to this?

test/api.test.ts Outdated
@@ -1,11 +1,12 @@
import {} from 'jest';
import {} from "jest";

Choose a reason for hiding this comment

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

Do we need this import? Seems like it's not being used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to change as little as possible. The quote change was just following the codestyle, IIRC.

@bowdenk7
Copy link
Contributor

hey @dmt - sorry I was away for a while.. but I'm back and going forward committed to keeping this repo fresh. I definitely want to take these changes. There are now merge conflicts from some other PRs I merged, so if you want to update this PR that'd be great. Otherwise I'll resolve them sometime this evening or tomorrow.

Apologies again for the long delay!

@dmt
Copy link
Contributor Author

dmt commented Dec 13, 2017

@bowdenk7 Thanks. I struggle a little remembering all the context. In particular the changes to server.ts while the contents moved to app.ts look like something I can't resolve without spending a bit more time getting back into it. So if you can resolve this, that would be appreciated.

nmchaves and others added 7 commits December 14, 2017 11:28
Added missing "node_modules/*" path mapping to README.
this seems a bit crude but allows running the tests and having them take
care of setting up the port to use
@bowdenk7 bowdenk7 merged commit ca7c9e5 into microsoft:master Dec 14, 2017
brittanydrandolph pushed a commit that referenced this pull request Jun 17, 2022
change tests to not always pass
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.

None yet

10 participants