ioBroker / testing

Shared testing utilities for ioBroker
MIT License
8 stars 12 forks source link

io pack test should check for `common.licenseInformation`instead of `common.license #592

Closed foxriver76 closed 6 months ago

foxriver76 commented 7 months ago
mcm1957 commented 7 months ago

Please ensure that tests accept both fields (common.license and common.licenseInformation) for now. Otherwise all existing adapters would fail :-). If both exist, ensure that license is identical

klein0r commented 6 months ago

My tests failed already 😄

  1) Validate the package files
       Check contents of io-package.json
         The property "common.license" exists:
     AssertionError: expected undefined not to be undefined
      at Context.<anonymous> (node_modules/@iobroker/testing/build/tests/packageFiles/index.js:61:57)
      at processImmediate (node:internal/timers:466:21)

  2) Validate the package files
       Compare contents of package.json and io-package.json
         The license matches:
     AssertionError: expected undefined to equal 'MIT'
      at Context.<anonymous> (node_modules/@iobroker/testing/build/tests/packageFiles/index.js:203:69)
      at processImmediate (node:internal/timers:466:21)
klein0r commented 6 months ago

Otherwise all existing adapters would fail

When the @iobroker/testing dependency is updated by the developer, the (new) test should fail.

mcm1957 commented 6 months ago

Test should accept both license and licenseInformation at least at the near future. Otherwise i.e. all dependabot PRs would fail.

There's absolutly no reason to migrate to licenseInformation for free adapters with high pressor. Failing tests yould be a big problem and cause complaints be developers. Admin will hanlde old license entry identical to free. Repository json will even drop licenseInfor mation for free adapters and add ist to json only if non-free. So I really do not see any reason to force devs to immidiatly react. A warnign at checker is OK to inform devs. But normal operation must not be blocked.

mcm1957 commented 6 months ago

My tests failed already 😄

  1) Validate the package files
       Check contents of io-package.json
         The property "common.license" exists:
     AssertionError: expected undefined not to be undefined
      at Context.<anonymous> (node_modules/@iobroker/testing/build/tests/packageFiles/index.js:61:57)
      at processImmediate (node:internal/timers:466:21)

  2) Validate the package files
       Compare contents of package.json and io-package.json
         The license matches:
     AssertionError: expected undefined to equal 'MIT'
      at Context.<anonymous> (node_modules/@iobroker/testing/build/tests/packageFiles/index.js:203:69)
      at processImmediate (node:internal/timers:466:21)

I've pinged Alcalzone to adapt tests

In the meantime I suggest to add but entries (license and licenseInformation). Should do no harm.

klein0r commented 6 months ago

In the meantime I suggest to add but entries (license and licenseInformation).

I've just updated all my adapters ... (removed the deprecated license and added the required licenseInformation)

foxriver76 commented 6 months ago

In the meantime I suggest to add but entries (license and licenseInformation).

I've just updated all my adapters ... (removed the deprecated license and added the required licenseInformation)

this is totally fine, repo build will transform the new attribute into the old ones (currently fixing some stuff on this, so may not yet be fully correct)

So if testing is fixed all fine.

foxriver76 commented 6 months ago

Fixing it shouldn't be too hard, we should remove this https://github.com/ioBroker/testing/blob/5078df7a1bc8394a7226e5edd751dfb817aca199/src/tests/packageFiles/index.ts#L98 and write a new test, which checks that one of both props is given, maybe log a warning if it is the old prop

https://github.com/ioBroker/testing/blob/5078df7a1bc8394a7226e5edd751dfb817aca199/src/tests/packageFiles/index.ts#L247 here we need to check first if license or licenseInfo contains the license

and maybe check additional that if it is licenseInfo it has correct structure. (if type not free, link is required)

mcm1957 commented 6 months ago

should be solved by PR https://github.com/ioBroker/testing/pull/594

klein0r commented 6 months ago

Still no new version? What's missing

mcm1957 commented 6 months ago

Did not yet receive a feedback (contacted @Alcazone at Telegram) and PR is not yet merged. But PR is only 3 days old.

@Apollon77 FYI