Skip to content

Conversation

@Jc2k
Copy link
Owner

@Jc2k Jc2k commented Jun 2, 2021

Implementation of #34

This will automatically replace stale reusable containers. For example:

  • Your co-worker has bumped a tag in an image() fixture, for example to use postgres 14 and its new JSON syntax. You git pull and your tests start failing. Sometimes this might not be obvious and the breakage might be some time after the root cause.
  • You make a change to some code that triggers a build() fixture. Right now the build() will still run. But the old container will be used.

@codecov
Copy link

codecov bot commented Jun 2, 2021

Codecov Report

Merging #35 (6b56bd9) into main (2456175) will increase coverage by 1.59%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   91.28%   92.88%   +1.59%     
==========================================
  Files          18       18              
  Lines         459      534      +75     
==========================================
+ Hits          419      496      +77     
+ Misses         40       38       -2     
Flag Coverage Δ
unittests 92.88% <100.00%> (+1.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pytest_docker_tools/factories/container.py 94.59% <100.00%> (+1.73%) ⬆️
pytest_docker_tools/factories/network.py 100.00% <100.00%> (ø)
pytest_docker_tools/factories/volume.py 100.00% <100.00%> (ø)
pytest_docker_tools/utils.py 98.21% <100.00%> (+1.91%) ⬆️
pytest_docker_tools/wrappers/container.py 82.63% <0.00%> (+0.69%) ⬆️
pytest_docker_tools/factories/image_or_build.py 100.00% <0.00%> (+14.28%) ⬆️

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 2456175...6b56bd9. Read the comment docs.

@Jc2k Jc2k marked this pull request as ready for review June 2, 2021 23:21
@Jc2k
Copy link
Owner Author

Jc2k commented Jun 3, 2021

@alexanderpetrenz Wanted to give you a heads up about this in case it causes any problems for your workflow. I need to do some "real" testing with it but I think i'll be merging it soon if that goes well.

@Jc2k
Copy link
Owner Author

Jc2k commented Jun 3, 2021

Manual integration tests:

venv with this branch installed.

Dockerfile:

FROM alpine:3.13
RUN apk --no-cache add python3
RUN mkdir /html
WORKDIR /html
EXPOSE 8000
CMD ["python3", "-m", "http.server"]
COPY index.html /html/index.html

index.html - really doesn't matter. Just junk that you can edit.

test_project.py

from pytest_docker_tools import build, container

myimage = build(
    path='.',
)

mycontainer = container(
    image='{myimage.id}',
    name='mycontainer',
    labels={'mylabel': 'test2'}
)

myothercontainer = container(
    image='{myimage.id}',
    name='myothercontainer',
    labels={'mylabel': '{mycontainer.id}'}
)

def test_1(myothercontainer):
    assert myothercontainer.name == 'myothercontainer'

This creates 2 containers. Then ran various changes and ran pytest --reuse-containers with each. In all cases containers were reused unless something was stale, in which case the correct things were recreated. For example:

  • Adding labels to myothercontainer only impacts myothercontainer.
  • Adding them to mycontainer impacts both.
  • Changing index.html causes a new image to build and both containers to be replaced

@Jc2k
Copy link
Owner Author

Jc2k commented Jun 3, 2021

  • Added a volume to myothercontainer, it (and only it) were detected as stale and recreated
  • Set initial_content on the volume. myothercontainer was redeployed
  • Changed initial_content. The container was redeployed.
  • Running the tests with no changes - container not redeployed
  • Added a label to the network, both containers were redeployed.

@Jc2k
Copy link
Owner Author

Jc2k commented Jun 3, 2021

Pretty pleased with how this is working. Test authors still have to be careful to not rely on state in the environment from previous test runs, but its now pretty safe and automatic if you have a fixture change. I'm probably going to merge + release this after lunch.

While not a breaking change code wise this does change the behaviour of reusable containers enough that i think it warrants another major version bump.

@Jc2k Jc2k merged commit 25ad943 into main Jun 3, 2021
@Jc2k Jc2k deleted the track_staleness branch June 3, 2021 10:21
@alpex8
Copy link
Contributor

alpex8 commented Jun 3, 2021

This is a great improvement! In fact I still wasn't able to hand over the whole reusable containers feature to colleagues, as they requested it month ago. It's great to see all this improvements so quickly. Thanks a lot for your work!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants