search

Wirecloud / docker-wirecloud

🐳 Docker Official Image packaging for WireCloud https://conwet.fi.upm.es/wirecloud
Other
8 stars 15 forks source link

Fix #32 - Add Docker Secrets support #33

Closed jason-fox closed 5 years ago

aarranz commented 5 years ago

Hi @jason-fox,

thank you very much for your contribution, you are doing a great work 😄.

But... seems something is not working as expected. I'm reviewing it to be able to provide you some feedback.

jason-fox commented 5 years ago

It looks to me as if the test suite is flaky. If I revert everything in the docker-entrypoint.sh ( commit 970de88 ) so the PR is documentation only then the dev branch is still failing. My only guess at the moment is a file permissions issue.

There is no reason why the suite should error and then swap to fail after some documentation only changes - weird.

aarranz commented 5 years ago

No, it's not a flaky test. Initial tests were inestable, I think, due internal network problems in the Travis network, as it was failing downloading pip packages and so on (I had to restart some of the Travis jobs).

But if you look at the Travis builds of the commits that you have made after my comment, they all fail with the same reason in very stable way:

======================================================================
FAIL: test_login_should_redirect_to_idm (__main__.IDMTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests.py", line 207, in test_login_should_redirect_to_idm
    self.assertEqual(response.status_code, 302)
AssertionError: 500 != 302

So the test is not flaky, but it is also true that your PR is not the one making this tests fail. It is due a change done on the WireCloud code and fixed on PR Wirecloud/wirecloud#400.

So... merging 😄.