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

Added support for proxy, updated packages and more. #21

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

vnysheoran
Copy link
Collaborator

Added these two new flags in command-line arguments

  -i, --inline-report-assets <n> Inline assets in generated HTML report.
  -p, --proxy [proxy]      The Proxy URL, e.g: http://127.0.0.1:8080

Content-length set to Zero when the body is empty.

.github/workflows/codeql-analysis.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
bin/openapitest.js Outdated Show resolved Hide resolved
bin/openapitest.js Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
src/superClient.js Show resolved Hide resolved
src/superClient.js Show resolved Hide resolved
src/superClient.js Show resolved Hide resolved
src/superClient.js Show resolved Hide resolved
mateusfreira
mateusfreira previously approved these changes Sep 9, 2021
Copy link
Collaborator

@mateusfreira mateusfreira left a comment

Choose a reason for hiding this comment

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

LGMT, thanks for the PR

Copy link
Collaborator

@dreich-nucleus dreich-nucleus left a comment

Choose a reason for hiding this comment

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

Small cleanup and I would like to know what inline does

bin/openapitest.js Outdated Show resolved Hide resolved
bin/openapitest.js Outdated Show resolved Hide resolved
@vnysheoran
Copy link
Collaborator Author

Addressed Changes requested by @dreich-nucleus

Copy link
Collaborator

@dreich-nucleus dreich-nucleus left a comment

Choose a reason for hiding this comment

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

The tags handling does not seem correct and you did not mention it in the PR description. I can't tell what I'm supposed to review here. But there are clearly bugs and changes needed.

@@ -18,6 +23,34 @@ if (apiPort.get('API_TESTS_PATH')) {
apiPort.currentFile = filePath;
const config = loadYamlFile(filePath);
describe(filePath, () => {
let tagsInTest = [];
if (tagsInTest) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is always true. Empty arrays are not "falsey". So, this block always executes. Just remove the if.

Unless you meant to check something that might be false - like the config.apiCalls.tag or tagsArray.length (remember tagArray will be truthy even when empty).

}
if (tagsArray.length > 0) {
let tagMatched = false;
for (const tagInArgs of tagsArray) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use lodash and one of its many set operators here - intersection is probably the one you want.

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.

None yet

4 participants