In Plea to game developers - NPC AI is good nowadays, but could it be better? I was rumbling about NPC AI in games. Well, I was reacting to game devs bragging left and right about how good AI in games is. It seems that this has been addressed in Tom Clancy's Splinter Cell: Blacklist which has been released month earlier in the same year, 2013 that is. At least according to GDC presentation Modeling AI Perception and Awareness in Splinter Cell: Blacklist. Kudos! I can only wish for other game studios to follow suit.

— Zdenek Styblik 2020/12/28 19:54

I've decided to replace simple dict with a dataclass in rss2irc PR#5, because dictionary is somewhat ambiguous and can be annoying at times. On the other hand, dataclass can be all of that(I'm sure) and is probably the closest as you can get to some kind of data structure in Python. Yes, you can use class(object) and that's fine by me. I know I do from time to time.

I have decided not to keep backwards compatibility and since I have several cache files, I've decided to write simple script to migrate data. I thought that having migration would be cool, although one could simply initialize cache files again at no cost. I have even written unit test to verify migration works as expected.

It did or seemed so, everything was green on my screen, merged, done. I've successfully migrated all cache files, no problem whatsoever. Cool! Now just to make sure everything really works with new format. Let's try to run scripts manually. rss2irc.py - no problems, everything is good. Other scripts, not so much. Why am I seeing what I'm seeing?

DEBUG:gh2slack:Traceback (most recent call last):
  File "./gh2slack.py", line 127, in main
    cache = rss2irc.read_cache(logger, args.cache)
  File "/root/scripts/rss2irc/rss2irc.py", line 202, in read_cache
    cache = pickle.load(fhandle)
AttributeError: Can't get attribute 'CachedData' on <module '__main__' from './gh2slack.py'>

I have written test for this and it was a PASS, therefore everything should work flawlessly. I've found Q&A at StackOverflow which suggested that class must be imported before use. It didn't make sense to me, but ok. Immediate fix(shortened version) looked like this:

diff --git a/gh2slack.py b/gh2slack.py
index 78fc7eb..d20504d 100755
--- a/gh2slack.py
+++ b/gh2slack.py
@@ -14,6 +14,7 @@ from typing import Dict, List, Set
 import requests
 import rss2irc
 import rss2slack
+from rss2irc import CachedData
 
 ALIASES = {
     'issues': 'issue',

And indeed, Traceback was gone which was great. However, I've decided to improve test coverage since this has been somehow missed and I didn't like that(not that rss2irc is used by somebody else and not that somebody is reading this crap). Let's cut to the chase. I was writing and rewriting wrong unit and integration tests, thus failing to replicate this issue.

• integration test for rss2slack.py executed as subprocess with new cache format
• execute migration, again as subprocess, and check cache in pytest

In the end, I've managed to reproduce issue by executing migration as subprocess and then execute small script which tries to read migrated cache. Lo and behold, this produced exactly the same Traceback.

The problem was the fact that I wanted migration to be frozen in time and work exactly the same even if dataclass is extended or changed in rss2irc.py. Therefore, I've duplicated(copy pasted) dataclass from rss2irc.py into migration script. However, pickle saves the name of the module as well. (Theory) This is fine for rss2irc.py, because module name, __main__, is same in given context, but not when executed from eg. rss2slack.py which calls rss2irc.read_cache(). Issue has been fixed in rss2irc PR#6 including proper unit test.

Take away is that unit and integration tests are not bulletproof. If tests are either written incorrectly or testing the wrong thing or wrong way, tests can give misleading results or provide false sense of security/confidence. Tests are the best effort, although this seems to be rather harsh and improper expression(I'm unable to think of better one, sorry). Anyway, is it worth writing tests then? Absolutely.

— Zdenek Styblik 2020/12/30 20:12

I've heard couple rumors that AWS Elastic Beanstalk(EBT) can end up belly up for no reason. It almost seemed like this can happen at random. Ok, seriously - this is one of pain points and drawbacks of EBT and deploys. I waived it off since no details were provided and when something like that happens, I'm going to deal with it. What else?

It did happen to us. Simple change like any other. Test at localhost was a OK and so were unit tests/CI. The same cannot be said about deploy. It failed, crashed, burned and left no return address. Deploy crashed in one of .ebextension/.platform hooks. Unfortunately, this was some time ago and I didn't keep the logs. This part was working before and was not touched by MR. However, that was the only lead(error) I could find in AWS CloudWatch logs. As usual this was misleading. Also, container was being killed so fast, one couldn't catch any logs. Enough of blabbering, database migration was to blame, because data couldn't be migrated for one reason or another, and there is /var/log/eb-docker/containers/eb-current-app/unexpected-quit.log you should check in case something like that happens. Note that there is a 50:50 chance rollback to previous version will be successful and happen(don't be surprised if it does not). It's probably somewhere in documentation, although I cannot find it right now. Anyway, nothing new since EBT is quite old platform and everybody knows this one.

— Zdenek Styblik 2020/12/31 19:40