[bitbake-devel] [PATCH v3 7/7] tests/fetch.py: add npm tests

[Jean-Marie LEMETAYER]

Hi Mark,

On Nov 20, 2019, at 7:29 PM, Mark Hatle mark.hatle at kernel.crashing.org wrote:
> First thanks for the tests!  A few comments inline
> 
> On 11/20/19 3:34 AM, Jean-Marie LEMETAYER wrote:
>> +
>> +    @skipIfNoNpm()
>> +    @skipIfNoNetwork()
>> +    def test_npm_no_network_no_tarball(self):
>> +        url =
>> 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
>> +        self.d.setVar('BB_NO_NETWORK', '1')
> 
> Since this is a BB_NO_NETWORK=1 case, the @skipIfNoNetwork() should not be here,
> since it should not need the network to perform the test.

Yes it is correct. In fact the @skipIfNoNpm() is not useful either as the
bb.fetch2.NetworkAccess exception should be raised before any npm commands.

>> +        fetcher = bb.fetch.Fetch([url], self.d)
>> +        with self.assertRaises(bb.fetch2.NetworkAccess):
>> +            fetcher.download()
>> +
>> +    @skipIfNoNpm()
>> +    @skipIfNoNetwork()
>> +    def test_npm_no_network_with_tarball(self):
>> +        url =
>> 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
>> +        fetcher = bb.fetch.Fetch([url], self.d)
>> +        fetcher.download()
>> +        self.d.setVar('BB_NO_NETWORK', '1')
> 
> Same here.
> 
> If any other the ones below work off local files, then similarly we shouldn't
> skip running them on a no network config.

As the DL_DIR point to a temporary directory created in the FetcherTest::setUp()
function when starting the test suite I think there is no way to work with local
files if you haven't download it first (which implies a network access).
Or did I miss something ?

>> +        fetcher = bb.fetch.Fetch([url], self.d)
>> +        fetcher.download()
>> +        fetcher.unpack(self.unpackdir)
>> +        unpackdir = os.path.join(self.unpackdir, 'npm')
>> +        self.assertTrue(os.path.exists(os.path.join(unpackdir,
>> 'package.json'))

Regards,
Jean-Marie