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

Refactor/tests #883

Merged
merged 2 commits into from
Nov 27, 2019
Merged

Refactor/tests #883

merged 2 commits into from
Nov 27, 2019

Conversation

orsagie
Copy link
Contributor

@orsagie orsagie commented Nov 27, 2019

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Refactor of tests

@orsagie orsagie requested a review from a team as a code owner November 27, 2019 08:34
@ghost ghost requested review from lili2311 and lwywoo November 27, 2019 08:35
@snyksec
Copy link

snyksec commented Nov 27, 2019

Messages
📖

This PR will not trigger a new version. It doesn't include any commit message with feat or fix.

Generated by 🚫 dangerJS against b7ea65c

@orsagie orsagie force-pushed the refactor/tests branch 2 times, most recently from d770956 to a3c64f9 Compare November 27, 2019 08:42
@dkontorovskyy
Copy link
Contributor

Maybe move cli-test.${ecosystem}.test.ts to cli-test folder and have ${ecosystem}.test.ts? Probably would be more readable.

@orsagie orsagie force-pushed the refactor/tests branch 3 times, most recently from 029ba0d to 6b084c5 Compare November 27, 2019 11:29
@orsagie orsagie force-pushed the refactor/tests branch 2 times, most recently from df75772 to db9eaf9 Compare November 27, 2019 11:32
@@ -1373,154 +1351,6 @@ test('`wizard` for unsupported package managers', async (t) => {
});
});

test('`protect` for unsupported package managers', async (t) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests are already in cli-protect.test.ts

@@ -949,29 +950,6 @@ test('`monitor golang-app --file=vendor/vendor.json`', async (t) => {
);
});

test('`test cocoapods-app (autodetect)`', async (t) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to cli-test

@@ -1535,51 +1365,6 @@ function stubExec(t, execOutputFile) {
});
}

test('error 401 handling', async (t) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are already in cli-test tests

const after = tap.runOnly ? only : test;

// Should be after `process.env` setup.
import * as plugins from '../../../src/lib/plugins/index';
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a nit peak, nothing more. Cos import will by default import index file. You can remove all the index occurrences

Copy link
Contributor

@dkontorovskyy dkontorovskyy left a comment

Choose a reason for hiding this comment

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

Apart from small nit pick, LGMT

@orsagie orsagie force-pushed the refactor/tests branch 2 times, most recently from 35a15d3 to cd6d0fb Compare November 27, 2019 12:20
@orsagie orsagie merged commit f92319f into master Nov 27, 2019
@orsagie orsagie deleted the refactor/tests branch November 27, 2019 13:43
@snyksec
Copy link

snyksec commented Nov 28, 2019

🎉 This PR is included in version 1.252.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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