search

collective / collective.cover

A sane, working, editor-friendly way of creating front pages and other composite pages. Working now, for mere mortals.
48 stars 55 forks source link

Use plone resource instead of browser resource #929

Closed wesleybl closed 2 years ago

wesleybl commented 2 years ago

Plone resource is better suited for the Resource Registry.

codecov-commenter commented 2 years ago

Codecov Report

Merging #929 (9d58657) into master (3f3bace) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #929   +/-   ##
=======================================
  Coverage   92.69%   92.69%           
=======================================
  Files          45       45           
  Lines        2835     2835           
=======================================
  Hits         2628     2628           
  Misses        207      207

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 3f3bace...9d58657. Read the comment docs.

fredvd commented 2 years ago

@wesleybl with other test setups there is always some output of the number of tests that have run and succes / failure numbers. But when I look at the GHA log output I can't find this, the only thing tox outputs is

''' Run tox -r py36-Plone52 create: /home/runner/work/collective.cover/collective.cover/.tox/py36-Plone52

tox: py36-Plone52 ___ summary ____ py36-Plone52: commands succeeded congratulations :) '''

Do I miss something and where? Or does tox actually run the python tests in the package?

mauritsvanrees commented 2 years ago

@fredvd There is a lot of output and GitHub Actions does not show it all by default. In the output below you have line 10, then 11, and then there is a jump to line 990:

Run tox -r
10 py36-Plone52 create: /home/runner/work/collective.cover/collective.cover/.tox/py36-Plone52
11 tox: py36-Plone52
990 ___________________________________ summary ____________________________________
991 py36-Plone52: commands succeeded
992 congratulations :)

If you click on line 11 the rest of the output is shown, ending with:

[989] Total: 273 tests, 0 failures, 0 errors and 0 skipped in 11 minutes 33.804 seconds.

So yes, it would be very useful if that extra line of output was included, but at least the tests are run correctly.

mauritsvanrees commented 2 years ago

I can't test this PR locally. During the buildout run I get this error:

error pixelsmith@2.6.0: The engine "node" is incompatible with this module. Expected version ">= 12.0.0". Got "8.11.2"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
ERROR: npm is known not to run on Node.js v8.11.2
You'll need to upgrade to a newer Node.js version in order to use this
version of npm. You can find the latest version at https://nodejs.org/

When seeing this error I made sure to have a recent enough node version with npm:

$ nvm use lts/fermium 
$ node --version
v14.19.3

Running buildout again gives the same error. It does not seem fatal: buildout seems to finish without error, with exit code zero. Startup fails though, because some files are missing that presumably should have been created by buildout/node/whatever:

zope.configuration.exceptions.ConfigurationError: ('No such file', '/Users/maurits/community/collective.cover/src/collective/cover/browser/static/resources.pt')
    File "/Users/maurits/community/collective.cover/parts/instance/etc/site.zcml", line 15.2-15.55
    File "/Users/maurits/community/collective.cover/parts/instance/etc/package-includes/002-collective.cover-configure.zcml", line 1.0-1.60
    File "/Users/maurits/community/collective.cover/src/collective/cover/configure.zcml", line 32.2-32.32
    File "/Users/maurits/community/collective.cover/src/collective/cover/browser/configure.zcml", line 156.2-164.8
wesleybl commented 2 years ago

@mauritsvanrees the buildout installs its own node. And it's pinned to node 14. See:

https://github.com/collective/collective.cover/blob/59e7e6b934991e2e79aa8aee21327667a43e8e5e/base.cfg#L65

What could have happened is that you run the buildout in a "dirty" directory, which had the old version of node, and the buildout didn't think it needed to update. You can remove the old version:

rm -rf parts/buildout-node/ parts/node/ webpack/node_modules/ .installed.cfg

And run the buildout again.