lirantal / lockfile-lint

Lint an npm or yarn lockfile to analyze and detect security issues
Apache License 2.0
781 stars 35 forks source link

fix(tests): update hosts validator tests #85

Closed bolatovumar closed 4 years ago

bolatovumar commented 4 years ago

Description

Some tests in validators.host.test.js are testing that the validate method doesn't throw when they should be testing that validation succeeds and the resulting validation object doesn't contain any errors.

Types of changes

Related Issue

Address #77

Motivation and Context

Improve tests

Checklist:

codecov-io commented 4 years ago

Codecov Report

Merging #85 into master will decrease coverage by 0.45%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
- Coverage   97.44%   96.98%   -0.46%     
==========================================
  Files          12        9       -3     
  Lines         235      166      -69     
  Branches       41       34       -7     
==========================================
- Hits          229      161      -68     
+ Misses          5        4       -1     
  Partials        1        1              
Impacted Files Coverage Δ
packages/lockfile-lint/src/validators/index.js
packages/lockfile-lint/src/config.js
packages/lockfile-lint/src/main.js

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d30ce73...e69f08d. Read the comment docs.

bolatovumar commented 4 years ago

@lirantal you will notice that one of the tests now fails. Specifically, it's this one:

it('validator should succeed if all resources are matching a host address', () => {
  const mockedPackages = {
    '@babel/code-frame': {
      resolved: 'https://registry.verdaccio.org/@babel/code-frame/-/code-frame-7.0.0.tgz'
    },
    meow: {
      resolved: 'https://registry.verdaccio.org/meow/-/meow-4.0.1.tgz'
    },
    '@babel/generator': {
      resolved: 'https://registry.verdaccio.org/@babel/generator/-/generator-7.4.4.tgz'
    }
  }

  const validator = new ValidatorHost({packages: mockedPackages})
  expect(validator.validate(['https://registry.verdaccio.org'])).toEqual({
    type: 'success',
    errors: []
  })
})

It was previously passing because validation doesn't throw but it doesn't succeed either. Is this supposed to succeed or not (i.e., should you be able to specify https://registry.verdaccio.org explicitly instead of just verdaccio)?

lirantal commented 4 years ago

@bolatovumar with regards to your previous message - I see all tests are passing now. Do we still need to discuss anything in this PR or ready to merge?

bolatovumar commented 4 years ago

@lirantal weird. One of tests still fails for me when I run it locally. It's the same one that I mentioned above. Could you try running that test locally to see if it fails for you as well?

● Validator: Host › validator should succeed if all resources are matching a host address

    expect(received).toEqual(expected) // deep equality

    - Expected
    + Received

      Object {
    -   "errors": Array [],
    -   "type": "success",
    +   "errors": Array [
    +     Object {
    +       "message": "detected invalid host(s) for package: @babel/code-frame
    +     expected: https://registry.verdaccio.org
    +     actual: registry.verdaccio.org
    + ",
    +       "package": "@babel/code-frame",
    +     },
    +     Object {
    +       "message": "detected invalid host(s) for package: meow
    +     expected: https://registry.verdaccio.org
    +     actual: registry.verdaccio.org
    + ",
    +       "package": "meow",
    +     },
    +     Object {
    +       "message": "detected invalid host(s) for package: @babel/generator
    +     expected: https://registry.verdaccio.org
    +     actual: registry.verdaccio.org
    + ",
    +       "package": "@babel/generator",
    +     },
    +   ],
    +   "type": "error",
      }

       97 | 
       98 |     const validator = new ValidatorHost({packages: mockedPackages})
    >  99 |     expect(validator.validate(['https://registry.verdaccio.org'])).toEqual({
          |                                                                    ^
      100 |       type: 'success',
      101 |       errors: []
      102 |     })

      at Object.toEqual (packages/lockfile-lint-api/__tests__/validators.host.test.js:99:68)
lirantal commented 4 years ago

Nope, locally all goes well. I'll go ahead and merge then.