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

Tests depend on `manifest.json` existing #1536

Closed
di opened this Issue Dec 7, 2016 · 13 comments

Comments

@di
Member

di commented Dec 7, 2016

Currently, running make tests from a clean checkout of Warehouse (namely, before trying to compile any static assets) will fail multiple tests because the file /app/warehouse/static/dist/manifest.json will not exist:

tests/conftest.py:144:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
warehouse/config.py:360: in configure
    strict=not prevent_http_cache,
warehouse/utils/static.py:20: in __init__
    super().__init__(*args, **kwargs)
/usr/local/lib/python3.5/site-packages/pyramid/static.py:263: in __init__
    self._manifest = self.get_manifest()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <warehouse.utils.static.ManifestCacheBuster object at 0x7fa897670320>

    def get_manifest(self):
>       with open(self.manifest_path, 'rb') as fp:
E       FileNotFoundError: [Errno 2] No such file or directory: '/app/warehouse/static/dist/manifest.json'

/usr/local/lib/python3.5/site-packages/pyramid/static.py:266: FileNotFoundError

In Travis, we're running gulp dist before running the test suite, so we're not seeing this issue there. Similarly, most developers have at some point compiled the static assets and built this file, so the tests don't fail.

We should either ensure that this file is built before running the test suite, or fix the tests to not depend on it existing.

@dstufft

This comment has been minimized.

Member

dstufft commented Dec 7, 2016

The tests generally rely on having make serve running don't they?

@dstufft

This comment has been minimized.

Member

dstufft commented Dec 7, 2016

(Maybe they don't and I'm a crazy person though!)

@di

This comment has been minimized.

Member

di commented Dec 7, 2016

You don't have to run make serve before running the make tests, if no containers are running, it will start the following:

Starting warehouse_rabbitmq_1
Starting warehouse_db_1
Starting warehouse_redis_1
Starting warehouse_elasticsearch_1

(but not the static container)

@di

This comment has been minimized.

Member

di commented Dec 7, 2016

We could fix this by adding static to the links for the web service, but I wasn't convinced we should necessarily have that dependency just for this file.

@di di added the bug 🐛 label Nov 30, 2017

@brainwane

This comment has been minimized.

Member

brainwane commented Dec 19, 2017

In our meeting today I asked us to double-check and we confirmed this isn't urgent enough for a near-term milestone.

@brainwane brainwane added this to the Cool but not urgent milestone Mar 2, 2018

brainwane added a commit to brainwane/warehouse that referenced this issue Mar 2, 2018

brainwane added a commit to brainwane/warehouse that referenced this issue Mar 4, 2018

brainwane added a commit to brainwane/warehouse that referenced this issue Mar 5, 2018

di added a commit that referenced this issue Mar 5, 2018

Document tests depending on `manifest.json` (#3120)
* Use Sphinx and rst markup

* Document current 'make tests' behavior

Workaround for #1536.
@di

This comment has been minimized.

Member

di commented May 14, 2018

I would really love it if someone could fix this during the packaging sprints!

@matt-land

This comment has been minimized.

Contributor

matt-land commented May 14, 2018

Would it be acceptable to test for the presence of manifest file, and "mark skipped" if the file does not exist yet?

@di

This comment has been minimized.

Member

di commented May 14, 2018

No, I think the test suite should properly mock out whatever requires this file to exist.

@Bjwebb

This comment has been minimized.

Contributor

Bjwebb commented Jul 28, 2018

@nlhkabu Could I take this issue please.

@di

This comment has been minimized.

Member

di commented Jul 28, 2018

@Bjwebb We have an open PR for this already: #3978

You can help by reviewing that PR though!

@xni

This comment has been minimized.

Contributor

xni commented Oct 27, 2018

@di, may be it makes sense to mark it for Bloomberg 2018 Event?

@shevelevs

This comment has been minimized.

shevelevs commented Oct 27, 2018

+1, sounds like an issue that could be addressed by https://github.com/orgs/pypa/projects/1

@di

This comment has been minimized.

Member

di commented Oct 27, 2018

Can someone create a new PR resolving the conflicts from #3978 and verify that it works?

PyPA Sprint Weekend at Bloomberg (2018) automation moved this from In progress to Closed Issues Nov 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment