fix Fifo queue delivers duplicate message(s)

[irahulranjan]

No description provided.

[coveralls]

Coverage increased (+0.001%) to 94.57% when pulling 8fb538a on irahulranjan:localstack_issue_3339 into effb075 on spulec:master.

[irahulranjan]

@BeyondEvil can you please help me out with the CI failure? I have run the tests in my machine using python 2.7, 3.6, 3.7, and 3.8 and all the tests are passing. I am having a hard time figuring out why it is failing in Travis.
Thanks..

[bblommers]

Hey @irahulranjan, the test are all failing in server mode, where Moto is run in a standalone Docker container.
The @mock.patch has no effect there, as it wouldn't change the value of DEDUPLICATION_TIME_IN_SECONDS inside the Docker container.

I don't really see how we would be able to test this inside a Docker container, so we can skip it in server mode like this:
https://github.com/spulec/moto/blob/master/tests/test_sqs/test_sqs.py#L1491

[irahulranjan]

@bblommers Thanks for the help.

[BeyondEvil]

Hey @irahulranjan, the test are all failing in server mode, where Moto is run in a standalone Docker container.
The @mock.patch has no effect there, as it wouldn't change the value of DEDUPLICATION_TIME_IN_SECONDS inside the Docker container.

I don't really see how we would be able to test this inside a Docker container, so we can skip it in server mode like this:
https://github.com/spulec/moto/blob/master/tests/test_sqs/test_sqs.py#L1491

Not exactly sure how the docker setup works, but you could change the DEDUPLICATION_TIME_IN_SECONDS to try and get it's value from the environment instead:

DEDUPLICATION_TIME_IN_SECONDS = os.getenv('DEDUPLICATION_TIME_IN_SECONDS', 300)

Then the test can set the env var to 1, either that will then get picked up by the docker runner automagically or it can be passed to the container when started.

Hope that makes sense @irahulranjan @bblommers

Edit:

Here you can add -e DEDUPLICATION_TIME_IN_SECONDS=1

Also, this is weird. It's first set to false and then to true? 🤔 🧐

[bblommers]

@BeyondEvil True, there are ways around it, but none of them pretty.
Changing the constant to read the environment variable means changing the code to make it more testable - which is always a code smell.
Passing the env-var to the Docker container means that value is set for the duration of the container lifetime - and we really only want to change it for the duration of a single test.

The weird travis.yml-notation is not setting the variable, but is used as a list of configurations. Travis will execute the tests once per configuration option (per python-version). That's why the list of test suites is executed twice per language, one with the env var set to True, one set to False.

[bblommers]

Thanks for the PR @irahulranjan!

[BeyondEvil]

@BeyondEvil True, there are ways around it, but none of them pretty.
Changing the constant to read the environment variable means changing the code to make it more testable - which is always a code smell.

Fair enough.

Passing the env-var to the Docker container means that value is set for the duration of the container lifetime - and we really only want to change it for the duration of a single test.

True.

It's already merged. But does moto support some type of configuration framework? I guess something like that could've been leveraged in a scenario like this.

The weird travis.yml-notation is not setting the variable, but is used as a list of configurations. Travis will execute the tests once per configuration option (per python-version). That's why the list of test suites is executed twice per language, one with the env var set to True, one set to False.

Oh, really. Wow, that is weird. 😅

Thanks for taking the time to explain. 🙏

[bblommers]

It's already merged. But does moto support some type of configuration framework? I guess something like that could've been leveraged in a scenario like this.

No, not yet! I agree that would be useful, and will become more useful as we're getting more config options. One day.. 🤞

[bblommers]

This is now part of moto >= 1.3.16.dev181