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

Add tests and more #47

Closed
wants to merge 9 commits into from
Closed

Add tests and more #47

wants to merge 9 commits into from

Conversation

sebcaps
Copy link
Contributor

@sebcaps sebcaps commented Dec 9, 2018

This PR aims to :

Please do not consider PR #45, since this one handles it.
I assume all unit test are Ok, but no clues if it will work with newman (especially URL's support) but according to #46 (comment), it should work like a charm...
I added radio to specify if one use file url or file, default is file for compatibility

@carlowahlstedt
Copy link
Owner

This is some great work!

I've added a few comments for things to consider. Additionally, the build is failing because:

2018-12-09T20:27:55.4665359Z > [email protected] compile D:\a\1\s
2018-12-09T20:27:55.4665699Z > tsc -p .
2018-12-09T20:27:55.4665799Z
2018-12-09T20:27:55.4665923Z Tests/customReporterTest.ts(29,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666072Z Tests/emptyCollectionTest.ts(25,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666205Z Tests/emptyContents.ts(25,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666367Z Tests/emptyEnvironmentFile.ts(24,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666515Z Tests/emptyEnvironmentURL.ts(24,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666638Z Tests/folderTest.ts(29,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666793Z Tests/globalVarFileTest.ts(30,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4666919Z Tests/globalVarTest.ts(29,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4667041Z Tests/noEnvironment.ts(24,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4667180Z Tests/numberOfIterationTest.ts(29,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4667304Z Tests/useCollectionURL.ts(25,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4667449Z Tests/useEnvironmentFile.ts(30,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4667573Z Tests/useEnvironmentURL.ts(26,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4668726Z Tests/wrongCollectionURL.ts(25,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.
2018-12-09T20:27:55.4668884Z Tests/wrongEnvironmentURL.ts(25,9): error TS2339: Property 'stats' does not exist on type 'TaskLibAnswers'.

I haven't dug into why it cannot find stats yet.

@sebcaps
Copy link
Contributor Author

sebcaps commented Dec 10, 2018

I absolutely don't understand this error.... As my dev friend often says 'it works on my machine'.
I'll have a check if I can do sthg but I suspect a diff between my machine and build agent...

@sebcaps
Copy link
Contributor Author

sebcaps commented Dec 10, 2018

Definitely no idea about compilation issue. @carlowahlstedt can you please have a look ?

@sebcaps
Copy link
Contributor Author

sebcaps commented Dec 13, 2018

Finally it's OK ;-)

@carlowahlstedt
Copy link
Owner

Not ignoring you here, just been really busy. I'll try to get back to looking at this over the weekend. I appreciate your work and patience on this.

.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
NewmanPostman/npm-debug.log Show resolved Hide resolved
@carlowahlstedt
Copy link
Owner

So...hadn't really used the PR changes before. I had started comments days ago, but didn't realize I had to submit them before you could see them. Either way, they are there now.

Overall everything looks really good and I think your tests are probably a big testament to that. I'm thinking we will release this to a new major version (so people have to manually upgrade) since there are several potential breaking changes here.

@sebcaps
Copy link
Contributor Author

sebcaps commented Dec 17, 2018

Ok with your comments.
I applied them in PR #49...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify collection using URL Remove mandatory environment file
2 participants