Refactoring: decomposed ScreenUpdater and Tester processors by DudaGod · Pull Request #712 · gemini-testing/gemini

DudaGod · 2017-01-12T21:28:12Z

Currently ScreenUpdater and Tester processors have the same logic about saving reference images.

Proposed changes:

create set of utils which implement particular functionality:
- RefChecker - checks exists reference or not
- Comparator - compare current and reference images
- Updater - save new reference image

DudaGod · 2017-01-13T06:29:18Z

lib/state-processor/capture-processor/utils/updater.js

+    static create(capture) {
+        return capture
+            ? new Updater(capture)
+            : new Updater();


кажется не лучшее решение, возможно красивее будет передавать capture в метод save вторым параметром

а зачем так делать?

почему не просто

return new Updater(capture);

?

DudaGod · 2017-01-13T06:41:14Z

test/unit/state-processor/capture-processor/tester.js

-            fs.accessAsync.returns(Promise.reject());
-            return assert.isRejected(tester.exec(capture, {}), NoRefImageError);
+                .then(() => assert.calledWith(imageStub.save, 'tmp/path'));
        });


тут тестов не хватает.
Нужно проверить еще, что компаратор не вызовется если refChecker.hasAccessTo вернет false и вызовется если вернет true.

tormozz48

В общем у меня есть существенное замечание про то, что использование классов в ходе данного рефактринга неоправданно. Лучше сделать отдельный модуль который бы экспортировал отдельные методы для проверки существования файла на файловой системе и пр.

tormozz48 · 2017-01-17T09:40:58Z

lib/state-processor/capture-processor/utils/comparator.js

@@ -0,0 +1,13 @@
+'use strict';


А тебе тут точно нужен класс?
У тебя же нет никакого внутреннего состояния

tormozz48 · 2017-01-17T09:42:15Z

lib/state-processor/capture-processor/utils/reference-checker.js

@@ -0,0 +1,16 @@
+'use strict';


Аналогичный вопрос по необходимость класса

tormozz48 · 2017-01-17T10:02:18Z

lib/state-processor/capture-processor/utils/reference-checker.js

+        return new RefChecker();
+    }
+
+    hasAccessTo(referencePath) {


Здесь ты по сути проверяешь существования файла на файловой системе.
И в название методы ты зашиваешь имя соответствующего нодового метода.

Это неправильно. Лучше назвать метод refExists или existsRef или existsRefImage

eGavr

@DudaGod

Возможно мы не совсем понимаем идею, которую вкладывал @j0tunn в необходимый рефакторинг.

Сейчас ты создал кучу классов типа comparator, reference-checker и т д, которым свой контекст this вообще не нужен, то есть все это можно вынести в отдельный файл utils как простые методы.

Давайте с Тохой это обсудим)

eGavr · 2017-01-17T08:46:06Z

lib/state-processor/capture-processor/utils/updater.js

+    }
+
+    copy(currentPath, referencePath) {
+        return fs.copyAsync(currentPath, referencePath).thenReturn(true);


не нравится то, что функция вдруг boolean возвращает.

Она и до этого boolean возвращала.
Так как вот тут - https://github.com/gemini-testing/gemini/blob/master/lib/state-processor/capture-processor/screen-updater/screen-updater.js#L16
нужно понимать обновился эталон или нет.

eGavr · 2017-01-17T08:46:13Z

lib/state-processor/capture-processor/utils/updater.js

+    save(referencePath) {
+        return fs.mkdirsAsync(path.dirname(referencePath))
+            .then(() => this._capture.image.save(referencePath))
+            .thenReturn(true);


и эта тоже.

eGavr · 2017-01-17T08:46:49Z

lib/state-processor/capture-processor/utils/updater.js

+    static create(capture) {
+        return capture
+            ? new Updater(capture)
+            : new Updater();


а зачем так делать?

почему не просто

return new Updater(capture);

?

eGavr · 2017-01-17T08:47:09Z

lib/state-processor/capture-processor/utils/updater.js

+
+    save(referencePath) {
+        return fs.mkdirsAsync(path.dirname(referencePath))
+            .then(() => this._capture.image.save(referencePath))


а если this._capture === undefined ?

tormozz48 · 2017-01-17T10:15:17Z

test/unit/state-processor/capture-processor/utils/updater.js

+
+    afterEach(() => sandbox.restore());
+
+    it('should have static factory creation method', () => {


Ну вот некоторые наши коллеги мне заворачивали такой тест по причине ненужности

tormozz48 · 2017-01-17T10:16:47Z

test/unit/state-processor/capture-processor/utils/updater.js

+            sandbox.stub(fs, 'copyAsync');
+        });
+
+        it('should save current image to reference path', () => {


save -> copy.
Ты же проверяешь и путь откуда копируется файл и путь назначения

tormozz48 · 2017-01-17T10:18:06Z

test/unit/state-processor/capture-processor/utils/updater.js

+        };
+
+        it('should make directory before saving the image', () => {
+            const mediator = sinon.spy().named('mediator');


А зачем так сложно?
Разве у тебя не отработает правильно без этого медиатора?

Так я же могу в коде просто написать:

fs.mkdirsAsync(refPath); capture.image.save(refPath);

Т.е. сохранение изображение запустится сразу же не дожидаясь создалась директория или нет и мой тест без медиатора пройдет. Так как стабы вызовутся в правильном порядке.

tormozz48 · 2017-01-17T10:19:47Z

test/unit/state-processor/capture-processor/utils/updater.js

+            fs.mkdirsAsync.returns(Promise.resolve());
+
+            return save_({refPath: '/ref/path'})
+                .then(() => {


в одну строчку

tormozz48 · 2017-01-17T10:22:00Z

test/unit/state-processor/capture-processor/utils/updater.js

+                });
+        });
+
+        it('should save image with correct path', () => {


with correct -> with given

eGavr

Посмотрел, по коду стало вроде лучше, но я не уверен, что прям реализована та идея рефакторинга, которую Антон вкладывал в ТЗ задачи.

eGavr · 2017-02-08T08:20:15Z

lib/state-processor/capture-processor/screen-updater/diff-screen-updater.js

+            canHaveCaret: capture.canHaveCaret,
+            tolerance: opts.tolerance,
+            pixelRatio: opts.pixelRatio
+        };


кстати таки один класс можно создать, так как эта логика используется в двух местах.

Ну типо создать класс Comparator, который в конструкторе принимает capture и opts и инициализирует эти опции.

eGavr · 2017-02-08T08:22:57Z

test/unit/state-processor/capture-processor/screen-updater/diff-screen-updater.js

+
+        const capture = {image: imageStub};
+        const DiffScreenUpdater = proxyquire(
+            'lib/state-processor/capture-processor/screen-updater/diff-screen-updater',


зачем proxyquire ?

eGavr · 2017-02-08T08:26:23Z

test/unit/state-processor/capture-processor/screen-updater/diff-screen-updater.js

+            });
+    });
+
+    it('should save current image with "png" extension', () => {


save a current

ниже тоже

название теста не соответствует тому, что проверяется в тесте.

Ты тут проверяешь, что temp.path вызывается с нужными аргментами, а не то, что картинка сохраняется.

То, что картинка сохраняется, ты проверяешь ниже в тесте

eGavr · 2017-02-08T08:26:50Z

test/unit/state-processor/capture-processor/screen-updater/diff-screen-updater.js

+
+    afterEach(() => sandbox.restore());
+
+    it('should not save current image if reference image does not exist', () => {


not save a current image if a reference image

eGavr · 2017-02-08T08:27:40Z

test/unit/state-processor/capture-processor/screen-updater/diff-screen-updater.js

+        return exec_().then(() => assert.calledWith(temp.path, {suffix: '.png'}));
+    });
+
+    it('should save current image to the temporary directory', () => {


to the temporary –> to a temporaty

eGavr · 2017-02-08T09:00:38Z

test/unit/state-processor/capture-processor/utils.js

+                .then(() => assert.callOrder(fs.mkdirsAsync, mediator, imageStub.save));
+        });
+
+        it('should save image with given path', () => {


with the given path

eGavr · 2017-02-08T09:01:03Z

test/unit/state-processor/capture-processor/utils.js

+        });
+
+        it('should save image with given path', () => {
+            fs.mkdirsAsync.returns(Promise.resolve());


тоже в beforeEach

eGavr · 2017-02-08T09:01:20Z

test/unit/state-processor/capture-processor/utils.js

+    });
+
+    describe('existsRef', () => {
+        it('should fulfill promise if reference image exists', () => {


should be fulfilled ...

eGavr · 2017-02-08T09:01:32Z

test/unit/state-processor/capture-processor/utils.js

+
+    describe('existsRef', () => {
+        it('should fulfill promise if reference image exists', () => {
+            fs.accessAsync.returns(Promise.resolve());


в beforeEach

eGavr · 2017-02-08T09:01:46Z

test/unit/state-processor/capture-processor/utils.js

+            return assert.isFulfilled(utils.existsRef('/existent/path'));
+        });
+
+        it('should reject with error if reference image does not exist', () => {


should be rejected with an error

j0tunn

Все равно ведь дублирование осталось. Моя мысль сводилась к тому, что у нас по сути есть всего две операции: проверка эталона и сравнение изображений, а различное поведение capture процессоров формируется в зависимости от реакции на эти две операции. Вот и хочется иметь какой-то набор функций/класов, а конкретный процессор формировать в фабрике под запрос. Типа такого:

exports.create = function(type) {
    if (type === 'tester') {
        return new CaptureProcessor()
            .onNoReference((refPath) => throw new NoRefImageError(refPath))
            .onEqual((refPath, currPath) => ({refPath, currPath, equal: true}))
            .onDiff((refPath, currPath) => ({refPath, currPath, equal: false}));
    }

    const saveRef = (refPath, capture) => utils.saveRef(refPath, capture);
    if (type === 'new-updater') {
        return new CaptureProcessor()
            .onNoReference(saveRef);
    }

    const updateRef = (refPath, currPath) => utils.copyImage(currPath, refPath);
    if (type === 'diff-updater') {
        return new CaptureProcessor()
            .onDiff(updateRef);
    }

    if (type === 'meta-updater') {
        return new CaptureProcessor()
            .onNoReference(saveRef)
            .onDiff(updateRef);
    }
}

А внутри CaptureProcessor уже реализуешь логику:

когда нужно позвать тот или иной хэндлер
выполняем сравнение (и зовем соответствующие хэндлеры) только если есть хоть один хэндлер onEqual или onDiff, и если существует эталон

Как-то так

j0tunn · 2017-03-03T20:59:45Z

lib/state-processor/capture-processor/screen-updater/diff-screen-updater.js

-                return fs.copyAsync(currentPath, referencePath).thenReturn(true);
+                return isEqual
+                    ? false
+                    : utils.copyImg(currentPath, referencePath);


.then((equal) => !equal && utils.copyImg(currentPath, refPath));

tormozz48 · 2017-06-18T17:41:29Z

lib/state-processor/capture-processor/factory.js

+        return utils.saveRef(referencePath, capture)
+            .then((updated) => ({imagePath: referencePath, updated}));
+    };
+    if (type === 'new-updater') {


Здесь лучше сделать пустую строку перед if

tormozz48 · 2017-06-18T17:52:16Z

lib/state-processor/capture-processor/factory.js

+        return utils.copyImg(currentPath, referencePath)
+            .then((updated) => ({imagePath: referencePath, updated}));
+    };
+    if (type === 'diff-updater') {


Здесь лучше сделать пустую строку перед if

tormozz48 · 2017-06-18T17:55:59Z

lib/state-processor/capture-processor/factory.js

+const CaptureProcessor = require('./');
+const utils = require('./utils');
+
+exports.create = (type) => {


Плохо читается:
Давай здесь перепишем по-другому:

const notUpdated = (referencePath) => ({imagePath: referencePath, updated: false}); const saveRef = (referencePath, capture) => { return utils.saveRef(referencePath, capture) .then((updated) => ({imagePath: referencePath, updated})); }; const updateRef = (referencePath, currentPath) => { return utils.copyImg(currentPath, referencePath) .then((updated) => ({imagePath: referencePath, updated})); }; exports.create = (type) => { if (type === 'tester') { .... } if (type === 'new-updater') { .... } if (type === 'diff-updater') { .... } if (type === 'meta-updater') { ..... } }

tormozz48 · 2017-06-18T18:05:42Z

lib/state-processor/capture-processor/utils.js

+exports.saveRef = (refPath, capture) => {
+    return fs.mkdirsAsync(path.dirname(refPath))
+        .then(() => capture.image.save(refPath))
+        .thenReturn(true)


Нет.
Давай не будем использовать вот такие штуки.

return fs.mkdirsAsync(path.dirname(refPath)) .then(() => capture.image.save(refPath)) .then(_.stubTrue) .catch(_.stubFalse)

Ну мне не очень нравятся эти стабы из лодаша. Давай тогда так напишем:

return fs.mkdirsAsync(path.dirname(refPath)) .then(() => capture.image.save(refPath)) .then(() => true) .catch(() => false)

tormozz48 · 2017-06-18T18:06:27Z

lib/state-processor/capture-processor/utils.js

+};
+
+exports.existsRef = (refPath) => fs.accessAsync(refPath)
+    .thenReturn(true)


См. комментарий выше

tormozz48 · 2017-06-18T18:08:34Z

lib/state-processor/update-state-processor.js

 };
+
+function identifyUpdaterType(opts) {
+    let type;


ИМХО лишняя переменная.
Ты же можешь писать вот так:

if (opts.diff && !opts.new) { return 'diff-updater'; } if (!opts.diff && opts.new) { return 'new-updater'; } return 'meta-updater';

tormozz48 · 2017-06-21T17:50:40Z

lib/state-processor/capture-processor/index.js

+        this._onDiffHandler = null;
+    }
+
+    onReference(handler) {


Хорошая идея. Я кстати такой подход использовал в dashboard при проектировании базового Job-а

tormozz48 · 2017-06-21T17:52:36Z

lib/state-processor/capture-processor/index.js

+        return utils.existsRef(referencePath)
+            .then((isRefExists) => {
+                if (!isRefExists) {
+                    return this._onNoRefHandler


Вот тут непонятно. При каких условиях у тебя может быть или не быть обработчика this._onNoRefHandler ? Не переусложнил ли ты в этом месте код?

А нет, все норм

по хорошему вместо проверок при вызове имеет смысл менять поведения объекта во время записи. Смотри, вызовы on* у тебя происходят только при создании объекта, а потом этот объект может несколько раз использоваться. В итоге можно просесть в производительности (да, в gemini у нас сейчас такие объекты одноразовые, но здесь тоже можно сделать какое-нибудь кэширование).
Т.е. лучше при построении создать объект, который уже никакие проверки внутри себя не делает

tormozz48 · 2017-06-21T17:58:33Z

lib/state-processor/capture-processor/index.js

+                        : Promise.reject(new NoRefImageError(referencePath));
+                }
+
+                if (!this._onEqualHandler && !this._onDiffHandler) {


Может быть здесь лучше написать вот так:

return (this._onEqualHandler && this._onDiffHandler) ? this._compareImages(capture, opts) : this._onRefHandler(referencePath);

//cc @DudaGod
А почему ты тут не исправил?

блин не заметил это замечание.
Спасибо, поправил.

tormozz48 · 2017-06-21T18:01:06Z

test/assert-ext.js

@@ -0,0 +1,4 @@
+global.assert.calledOnceWith = function() {


tormozz48 · 2017-06-21T18:02:36Z

test/unit/state-processor/capture-processor/index.js

+            return processor.exec(capture, opts);
+        };
+    };
+    let exec_;


Отбей пустой строкой

tormozz48 · 2017-06-21T18:29:49Z

test/unit/state-processor/capture-processor/index.js

+            return assert.isRejected(exec_({refPath: '/non-existent/path'}), NoRefImageError);
+        });
+
+        describe('should return image comparison result', () => {


Можно было бы вот так:

describe('should return image comparison result if images are', () => { it('equal', () => {}); it('different', () => {}); });

И вообще здесь не совсем понятно, что проверяется:

assert.deepEqual(result, { currentPath: '/temp/path', referencePath: '/ref/path', equal: true });

Предлагаю разбить на 3 теста:

Ты проверяешь, что есть currentPath: '/temp/path', и referencePath: '/ref/path'` (а может быть даже в 2-х отдельных тестах)

Проверяешь, что equal=true для одинаковых изображений.

Проверяешь, что equal=false для разных изображений.

Я тут сразу проверяю весь результат который возвращается при сравнении изображений. Это позволяет быть уверенным, что объект не содержит каких то новых полей. Не уверен, что его нужно разбивать так. В итоге же получится везде одинаковый вызов, но проверка по кусочкам. Тем более тут же тест не сложный получается.

tormozz48 · 2017-06-21T18:36:01Z

test/unit/state-processor/capture-processor/index.js

+            exec_ = mkExecMethod(newUpdater);
+        });
+
+        it('should not save a reference image if it is already exists', () => {


Ну приехали.
Ты проверяешь, что эталон не сохраняется и для этого у тебя есть assert.notCalled(utils.saveRef);
Зачем второй assert ?

Или второй ассерт тоже "в тему"?

Да, все-таки твой вариант правильный

tormozz48 · 2017-06-21T18:36:20Z

test/unit/state-processor/capture-processor/index.js

+                });
+        });
+
+        it('should save a reference image if it does not exist', () => {


См. замечание выше

tormozz48 · 2017-06-21T18:41:49Z

test/unit/state-processor/capture-processor/tester.js

-        });
-    });
-});
+// 'use strict';


А это что за закоментированный код ?

забыл удалить. Удалил

tormozz48 · 2017-06-21T18:43:36Z

test/unit/state-processor/capture-processor/utils.js

+        it('should make a directory before saving the image', () => {
+            const mediator = sinon.spy().named('mediator');
+
+            fs.mkdirsAsync.callsFake(() => Promise.delay(1).then(mediator));


И опять непонятно про медиатор

выше писал, для чего так делаю

j0tunn · 2017-07-17T15:31:56Z

lib/state-processor/capture-processor/factory.js

@@ -0,0 +1,44 @@
+'use strict';


я обычно фабрику кладу в index.js, а базовый класс в одноименный файл, т.е. можно переименовать: factory.js -> index.js, index.js -> capture-processor.j.
По большому счету входная точка-то здесь как раз фабрика, т.е. правильно будет просто подключить папку: require('./capture-processor')

j0tunn · 2017-07-17T15:51:52Z

lib/state-processor/capture-processor/index.js

+        return utils.existsRef(referencePath)
+            .then((isRefExists) => {
+                if (!isRefExists) {
+                    return this._onNoRefHandler


по хорошему вместо проверок при вызове имеет смысл менять поведения объекта во время записи. Смотри, вызовы on* у тебя происходят только при создании объекта, а потом этот объект может несколько раз использоваться. В итоге можно просесть в производительности (да, в gemini у нас сейчас такие объекты одноразовые, но здесь тоже можно сделать какое-нибудь кэширование).
Т.е. лучше при построении создать объект, который уже никакие проверки внутри себя не делает

j0tunn · 2017-07-17T15:56:51Z

lib/state-processor/test-state-processor.js

        super({
-            module: require.resolve('./capture-processor/tester'),
-            constructorArg: config.system.diffColor
+            module: require.resolve('./capture-processor/factory'),


родителю теперь достаточно передать только тип процессора, и можно обойтись без объекта

j0tunn · 2017-07-17T15:58:57Z

lib/state-processor/update-state-processor.js

+        return 'new-updater';
+    }
+
+    return 'meta-updater';


return opts.diff && !opts.new && 'diff-updater' || !opts.diff && opts.new && 'new-updater' || 'meta-updater'

и можно в функцию не выносить

j0tunn

Еще файлы, которые затрагивал, на es6 переведи, и норм.

j0tunn · 2017-08-22T15:26:33Z

lib/state-processor/capture-processor/index.js

+    if (type === 'diff-updater') {
+        return CaptureProcessor.create()
+            .onReference(_.noop)
+            .onNoReference(notUpdated)


а мы сейчас разве в этом случае не бросаем ошибку?

Нет - https://github.com/gemini-testing/gemini/blob/master/lib/state-processor/capture-processor/screen-updater/diff-screen-updater.js#L12
Перед тем как влить еще раз внимательно проверю, что поведение не изменилось при вызове с любой из опций + прогоню интеграционно

j0tunn · 2017-08-22T15:30:03Z

lib/state-processor/capture-processor/index.js

+exports.create = (type) => {
+    if (type === 'tester') {
+        return CaptureProcessor.create()
+            .onReference(_.noop)


можно по идее по умолчанию в _onRefHandler записать _.noop (ну или даже во все хэндлеры по умолчанию записать _.noop). Тогда тебе вот это писать не нужно будет

j0tunn · 2017-08-22T15:34:40Z

lib/state-processor/update-state-processor.js

    }
 };
+
+function identifyUpdaterType(opts) {


ну я бы не выносил это в отдельную функцию. Просто в конструкторе сохранил бы переменную updaterType

levonet added the in progress label Jan 12, 2017

DudaGod commented Jan 13, 2017

View reviewed changes

tormozz48 suggested changes Jan 17, 2017

View reviewed changes

eGavr reviewed Jan 17, 2017

View reviewed changes

tormozz48 suggested changes Jan 18, 2017

View reviewed changes

DudaGod force-pushed the refactor/screen-updater branch 3 times, most recently from 40e692f to cdcf946 Compare February 8, 2017 05:07

eGavr reviewed Feb 8, 2017

View reviewed changes

j0tunn suggested changes Mar 4, 2017

View reviewed changes

DudaGod force-pushed the refactor/screen-updater branch from 8ea2ef0 to 4d65706 Compare June 6, 2017 07:11

tormozz48 suggested changes Jun 18, 2017

View reviewed changes

tormozz48 suggested changes Jun 21, 2017

View reviewed changes

tormozz48 approved these changes Jun 22, 2017

View reviewed changes

DudaGod force-pushed the refactor/screen-updater branch from d2ca22b to 90b67cf Compare June 22, 2017 08:08

j0tunn suggested changes Jul 17, 2017

View reviewed changes

DudaGod force-pushed the refactor/screen-updater branch 2 times, most recently from 432895a to a1fffe5 Compare July 22, 2017 22:28

j0tunn approved these changes Aug 22, 2017

View reviewed changes

DudaGod force-pushed the refactor/screen-updater branch 2 times, most recently from 5c4ca22 to ffea13b Compare August 23, 2017 04:51

DudaGod added 3 commits August 23, 2017 08:29

refactor: decomposed ScreenUpdater and Tester processors

55f19b0

test: rewrite old tests and add new ones

a6eee4e

test: add method 'calledOnceWith' to 'assert'

9fa38b5

DudaGod force-pushed the refactor/screen-updater branch 2 times, most recently from ec225cf to 9fa38b5 Compare August 28, 2017 07:12

DudaGod merged commit 39c5352 into master Aug 28, 2017

DudaGod deleted the refactor/screen-updater branch August 28, 2017 21:57

levonet removed the in progress label Aug 28, 2017


		afterEach(() => sandbox.restore());

		it('should have static factory creation method', () => {


		afterEach(() => sandbox.restore());

		it('should not save current image if reference image does not exist', () => {

Comments

Conversation

DudaGod commented Jan 12, 2017

Proposed changes:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tormozz48 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eGavr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eGavr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

j0tunn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

eGavr left a comment •

edited

Loading

eGavr left a comment •

edited

Loading