Pull to refresh

Comments 61

Почему бы не использовать какие-нибудь event? И контролировать, что он взводится финализатором (вставить ожидание на 1 сек, например). Или named-pipes? Или вообще, пусть финализатор в созданный тестом файл пишет какой-то свой идентификатор и контролировать содержимое файла, а удалять его будет тест? Или иам нужно (тестируется) именно создание/удаление файла?
Я хотел написать тест для двух целей: с одной стороны убедиться, что финализатор вызывает удаление файла, а с другой стороны убедиться, что файл будет удален. Ваши предложения хороши для первой части, но главная трудность тут во второй.
Не понимаю, почему вы это называете словами «как проверить код финализатора»??

Вы помимо кода финализатора хотите проверить:

1) Механизм виртуальной машины который вызывает финализатор (не доверяете майкрософту? думаете они это не тестируют?).
2) Механизм базовых библиотек которые удаляют файлы (тоже какое то неадекватное недоверие).

Если хотите проверить работоспособность своего кода, который содержит значимый бизнес, а не вызов Dispose, то думаю вы без проблем сумеете его вызвать.

По факту вы хотите воссоздать какое то окружение, близкое к реальной ситуации и проверить не свой код, а его связку с базовыми механизмами — вас спасут системные тесты на весь продукт в целом. Интеграционный тест на такую ситуацию написать трудно — да и смысла в этом нет — все равно это будет не то же самое, что в реальном окружении.
В общем простите — но по моему вы ерундой какой то занимаетесь. Правда — трудно поверить что такой тест может иметь смысл.
Может я не правильно выразился, но естественно, я хочу проверять свой код, и даже если он написан в финализаторе.
Работа с базовыми библиотеками (работа с файлами), требует правильного вызова (включая правильную последовательность), а это уже мой код, который я опять же хочу проверить.
Мне кажется вы не совсем понимаете сути. Ваш код — это ваш написанный код. Все что не ваш код — это не ваш код. Его не нужно проверять если речь идет о модульных тестах.

То поток выполнения кода не должен выходить за пределы вашего кода. Внешним интерфейсом, поведение которого вы проверяете будет сигнатура ваших методов (сигнатура метода, вызываемого из финализатора например) и все точки вызовов внешних библиотек (моки). Используя язык моков вы и последовательность внешних вызовов можете проверить и то, с какими параметрами они вызываются.

Тестировать приватные методы (которые внутри финализатора вызываются) не проблема совершенно. Мокать статические классы базовых библиотек — тоже не большая проблема — для этого есть специальные инструменты. Можно обойтись даже без своих оберток.

Тестировать то, что отрабатывает собственно финализатор и к чему это приводит нужно совсем другими тестами и в рамках другой деятельности (системное тестирование приложения как черного ящика).
А если речь идет не о модульных тестах?
Модульный тест проверят, что мой код вызвал удаление файла.
Интеграционный тест должен проверять, что файл действительно был удален.

Ок — а зачем проверять, что файл был действительно удален. Вашего уровня доверия базовым библиотекам недостаточно для того, чтобы быть уверенным в том, что если вы говорите им «удали файл» — то библиотека будет себя вести в соответствии со спецификацией?

Ведь создатели библиотеки уже потратили деньги на ее тестирование.
Мне важно проверять, что файл был действительно удален, потому что, может быть, что я открыл файл в одном месте и пытаюсь удалить до того, как я его закрыл.
1. Возможно для решения проблемы вам подойдет вот этот хитроспрятанный класс TempFileCollection.
2. В предыдущей статье, уважаемый mayorovp уже упоминал возможность удаления всех временных файлов при закрытии процесса, но там велась речь о решении с помощью P/Invoke. На самом деле есть стандартная обертка над этим флагом.

P.S.
По моему скромному мнению, код вашего финализатора тестировать не стоит(по крайней мере с помощью unit-теста).
Между прочим, файлы удаляются не при закрытии процесса, а при закрытии их дескриптора. То есть если этот дескриптор обернуть в SafeHandle — то получится то же самое, что сделал автор статьи в своем классе…
Спасибо за наводки. Надо будет изучить.

2. В предыдущей статье, уважаемый mayorovp уже упоминал возможность удаления всех временных файлов при закрытии процесса, но там велась речь о решении с помощью P/Invoke. На самом деле есть стандартная обертка над этим флагом.


Не совсем понимаю, как это может помочь в случае использования Path.GetTempFileName. Не хотелось бы переходить на собственную реализация «временных» файлов.

По моему скромному мнению, код вашего финализатора тестировать не стоит(по крайней мере с помощью unit-теста).


Готов написать два теста: unit тест для проверки, что вызывается удаление файла, но мне нужет еще тест для проверки того, что файл действительно удаляется.

Кстати, «тестировать не стоит» потому что это тестируется глазками, или потому что, нет нормального способа?

Не совсем понимаю, как это может помочь в случае использования Path.GetTempFileName. Не хотелось бы переходить на собственную реализация «временных» файлов.

Возможно вам стоит отказаться от использования «Path.GetTempFileName» и создавать файл вручную с необходимыми флагами.

Готов написать два теста: unit тест для проверки, что вызывается удаление файла, но мне нужет еще тест для проверки того, что файл действительно удаляется.

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

Кстати, «тестировать не стоит» потому что это тестируется глазками, или потому что, нет нормального способа?

Мне кажется, что тест проверяющий сам факт вызова метода «File.Delete» является достаточным для модульного(unit) теста.
Ну и, например лично мне, не нравится когда сложность теста, намного превышает порядок сложность тестируемого и кода.
Полностью со всем согласен.

Интересно, стоит ли писать модульный тест для проверки удаления временного файла? По моему мнению да. Тогда интересно как этот тест может быть реализован?

Кстати, я понимаю, что тест сложен (по сравнению с кодом), но моя цель избежать сложных проблем при длительном реальном использовании программы, и это требует, в данном случае, сложный тест.
Почитал я про TempFileCollection и немного удивился тому, что
This method can be called only once for each file name extension, because it will return the same name if it is called with the same extension.
.

Нельзя создать два временных файла с одним и тем же расширением? Если да, то это сильно ограничивает область использования TempFileCollection.
У меня такие мысли:

1. Необёрнутый в try-catch вызов File.Delete в Dispose — плохо. Файл может отказаться удалиться по тем или иным причинам, а Dispose не должен (should not) кидать исключения.

2. В чём смысл тестирования единственной строки Dispose(false) — а именно таким обычно и бывает финализатор? Хотите протестировать финализатор? Тестируйте Dispose(false).

3.
мне нужет еще тест для проверки того, что файл действительно удаляется
Вам не нужно тестировать File.Delete. Это уже не ваша забота. Что стоит протестировать — это факт того, что File.Delete вызывается. Для этого в Visual Studio есть Fake Assemblies.

4. Unit-тест, зависимый от внешней среды (в вашем случае — файловой системы), плох.
Прикольная реализация :)

 void Delete(string fileName) {
            try {
                File.Delete(fileName);
            }
            catch {
                // Ignore all exceptions
            }
        }
Там больше интересна реализация проверки того, что файл был создан:

 void EnsureTempNameCreated() {
            if (basePath == null) {
 
                string tempFileName = null;
                FileStream tempFileStream;
                bool uniqueFile = false;
                int retryCount = 5000;
                do {
                    try {
                        basePath = GetTempFileName(TempDir);
 
                        string full = Path.GetFullPath(basePath);
 
                        new FileIOPermission(FileIOPermissionAccess.AllAccess, full).Demand();
 
                        // make sure the filename is unique. 
                        tempFileName = basePath + ".tmp";
                        using (tempFileStream = new FileStream(tempFileName, FileMode.CreateNew, FileAccess.Write)) { }
                        uniqueFile = true;
                    }
                    catch (IOException e) {
                        retryCount--;
 
                        uint HR_ERROR_FILE_EXISTS = unchecked(((uint)0x80070000) | NativeMethods.ERROR_FILE_EXISTS);
                        if (retryCount == 0 || Marshal.GetHRForException(e) != HR_ERROR_FILE_EXISTS)
                            throw;
 
                        uniqueFile = false;
                    }
                }while (!uniqueFile);
                files.Add(tempFileName, keepFiles);
                
            }
        }


Обратите внимание на цикл while.
Т.е. надо и производительность проверить и сравнить с GUID реализацией?
Или эту работу уже кто-нибудь сделал?
А как часто у вас создаются временные файлы?
Как то не задавался этим вопросом. Думаю, что в общем случае не часто. По крайней мере профайлинг давно не указывал на работу с файлами.
2. В чём смысл тестирования единственной строки Dispose(false) — а именно таким обычно и бывает финализатор? Хотите протестировать финализатор? Тестируйте Dispose(false).


Смысл тестирования, убедиться (и убеждаться все время и при всяких изменениях), что финализатор вызывает удаление файла.
Финализатор (при правильной реализации IDisposable pattern) должен (should) вызывать Dispose(false). Этот факт можно тестировать статическим анализом. Соответственно, если Dispose(false) вызывает удаление файла (вы же написали тест?), то можно быть уверенным, что и финализатор тоже вызовет удаление файла, unit-тест излишен.
Этот факт можно тестировать статическим анализом.

Интересный подход. Можете посоветовать, что то конкретное? Какой инструмент и как эту проверку задать?
и как задать проверку правильности финализатора?
Project -> Properties -> Enable Code Analysis on Build.
Поизучайте Rule Sets. Нажмите кнопку Open, посмотрите правила.
CA1063 находится в Microsoft.Design. Там же CA1065.
CA1063 хорошая вещь, но в данном случае слишком сильная: я бы начал с финализатора, а правило CA1063 проверяет и Dispose() тоже.
Кстати, а какие есть способы добавить свою статическую проверку (правило)? Microsoft Code Analysis вроде бы не позволяет. Может есть плагин для R#? Или можно самому такой реализовать. У меня есть желание анализировать наши кастомные штуки, но вот пока не знаю как (приходится простым поиском и глазками).
Камрад jonie утверждает, что написание всевозможных плагинов упростилось с введением в строй проекта Roslyn. Возможно он сможет поделиться литературой или даже кодом.
Дак поставьте Roslyn SDK — там есть шаблон проекта. И да: это пока работает на VS14 «из коробки». В любом случае можно нарисовать не плагин, а отдельную утилиту, взяв roslyn как парсер…
А вот тут я подсказать, к сожалению, уже не смогу. Погуглите по ключевым словам [writing|creating] custom fxcop rule.
4. Unit-тест, зависимый от внешней среды (в вашем случае — файловой системы), плох.

Согласен, поэтому я и написал, что допустим unit-тест перепишу, но интересно обсудить как написать тест, который проверит удаление файла.
3.
мне нужет еще тест для проверки того, что файл действительно удаляется
Вам не нужно тестировать File.Delete. Это уже не ваша забота. Что стоит протестировать — это факт того, что File.Delete вызывается. Для этого в Visual Studio есть Fake Assemblies.


Для unit-теста это не моя забота. Опять согласен.
Но для более другого теста, это моя забота. Мне нужно быть уверенным, что финализатор не только вызовет удаление файла, но и удалит его.
Какой-такой «более другой» тест?

Более того, File.Delete уже оттестирован, и поведение метода детерминировано: либо файл действительно удалится, либо выкинут исключение. Вам достаточно проверить, что ваш код вызывает File.Delete и съедает все исключения. Не надо придумывать ничего сверх этого.
Вам достаточно проверить, что ваш код вызывает File.Delete и съедает все исключения

К сожалению не всегда достаточно. Мне очень важно убедиться, что файл действительно будет удален. Может быть мой код открыл файл и не закрыл, и тогда тест должен упасть, иначе проблема будет скрыта и обнаружется только тогда, когда кончаться временные файлы
Но это уже тест не самого класса MyTemporaryFile, а тест классов, его использующих. Можно в классе MyTemporaryFile ввести статический счетчик исключений, возникших при удалении файла — а в тестах проверять, что значение этого счетчика не изменилось в процессе выполнения теста. Точно так же можно проверять факт вызова Dispose.
Понимаете, в большинстве случаев код пишется с учётом определённых ожиданий от внешней системы. Например, вы пишете var x = new Form1(); и ожидаете, что есть свободная память, она читаема и не сбоит. Если вы пишете File.Delete(...), то вы ожидаете, что файл действительно будет удалён, либо выкинется исключение. С первым случаем, думаю, все понятно. Вот вам второй случай: выкинулось UnauthorizedAccessException (злобный пользователь нашёл ваш временный файл и поставил ему атрибут read only), файл не удалён. Какой вы сделаете вывод? Правилен ли ваш код? Должен ли валиться тест?
Я согласен, что есть внешние обстоятельства, которые проверять не очень можно. Но есть сценарий, где есть место и для тестов.
Например, если моя обертка «временный файл», открывыает файл в конструкторе и должна его закрыть перед финализатором.
В этом случае, проверка того, что финализатор вызывает удаление файла хорошо для unit теста, но не достаточна для интеграционного теста. Мне важно убедиться, что файл будет удален.

Если попробовать обобщить, то есть некий внешний ресурс, с которым работает обертка и важно убедиться, что эта работа корректная (с точки зрения внешнего ресурса, причем не всегда мы знаем как он себя ведет).
Ну вот ответьте на мои вопросы. Они не риторические :)
на тестовой машине нет пользователей (не злобных, не добрых). добрый этот, который снимает read-only после злобного :).
а программистам я доверяю.
поэтому пусть тест валится.
Но ведь код правильный — он пытался удалить файл. Просто случился форс-мажор в виде флажка read only, обстоятельство непреодолимой силы, так сказать.
Вы не понимаете. Автор пытается отловить ситуацию, когда в форс-мажоре виновата сама же программа, которая еще не закрыла файл, но уже пытается его удалить.

Кстати, в случае с FileOptions.DeleteOnClose такое невозможно — еще один повод автору перейти на использование этого флага.
А-а, да? Ну, тогда все эти танцы, наверное, определённый смысл имеют.
По крайней мере, мне на ум приходит как минимум один вариант, где финализатор выполняет нетривиальную (à la вызвать Dispose(false)) работу — TaskScheduler.UnobservedTaskException.
когда кончатся временные файлы
И объясните, пожалуйста, как могут кончиться временные файлы? :) 65535 файлов — это ограничение Path.GetTempFileName, но никто не может вам запретить создавать временные файлы другим способом.
Я поддержу мнение withkittens, возможно вам стоит использовать какой-либо другой способ создания временных файлов. Ведь «Path.GetTempFileName» это просто удобная обертка, которая работает в 99% сценариев, но если она накладывает неприемлимые для вас ограничения, тогда от неё стоит отказаться.

Действительно, она накладывает ограничения на количество файлов, что и заставляет меня искать причины, почему временные файлы не удалятся. Один из способов защиты — это тестирования обертки «временный файл».
Мне кажется, что вы сами и ответили: если использовать Path.GetTempFileName, то временные файлы кончатся. И такие ситуации я встречал на практике.

Я понимаю, что один из способов это использовать самопальные временные файлы, что теоретически (в случае, если удаление реально не выполняется), только увеличивает время жизни процесса, до того, как кончится место на диске :).

Недавно чистил временную папку. Так было что почистить :).

Ну что значит «самопальные»? Path.GetTempFileName — просто обёртка над достаточно глупой функцией, она ничем не лучше собственных временных файлов. Наоборот, а) вы сможете использовать более гибкие имена (например, гуиды; они не кончатся ;), б) вы сможете лучше контролировать время жизни файлов: вместо танцев с тестами финализатора создавайте файлы с FileOptions.DeleteOnCloseсама ОС позаботится, что файл будет удалён, даже если ваш процесс вообще упадёт с ошибкой.
Вообще то я использую и реализацию временных файлов через GUID. Когда мне понадобился временный файл с определенным расширением мне пришлось перейти на GUID.

б) вы сможете лучше контролировать время жизни файлов: вместо танцев с тестами финализатора создавайте файлы с FileOptions.DeleteOnClose — сама ОС позаботится, что файл будет удалён, даже если ваш процесс вообще упадёт с ошибкой.


FileOptions.DeleteOnClose() существенное преимущество над Path.GetTempFileName(). Надо будет воспользоваться этой идеей. Но, как мне кажется, принципиально это ничего не меняет.
Именно что меняет. Можно использовать готовый финализатор, скрытый внутри SafeHandle — в вашем же коде финализатора не будет. А значит, и тестировать нечего.

Вам же остается только «пробросить» вызов Dispose внутрь SafeHandle — но Dispose тестируется элементарно.
ок. Это действительно может поменять все дело. Буду проверять.
Спасибо.
Вам не нужно тестировать File.Delete. Это уже не ваша забота. Что стоит протестировать — это факт того, что File.Delete вызывается. Для этого в Visual Studio есть Fake Assemblies.


Что бы использовать Fake Assemblies, мне нужно иметь тестовый проект от Microsoft или достаточно обычного проекта с nunit тестами?
Мне кажется у вас неверно поставлена задача. Как вам уже отвечали в прошлой статье, нет никаких гарантий что финализатор вообще отработает, а если и отработает, то никто, даже сама платформа, не знает когда он отработает.
Как мне помнится, сборщик мусора один на все, и к тому моменту, как у вас подходит дело к проверкt а удален ли файл, финализатор может вообще еще не отработал, и это не значит, что код сломался. Просто еще время не пришло. Если вам нужно обязательно удалять файл, тогда требуется использовать патерн Disposable и явным образом вызывать метод Dispose.
use (var file = new TempFile()){
SomeOperations();
}
И я полностью согласен с комментарием
Вам не нужно тестировать File.Delete. Это уже не ваша забота. Что стоит протестировать — это факт того, что File.Delete вызывается. Для этого в Visual Studio есть Fake Assemblies.
пользователя withkittens.
Подробно о сборке мусора в таком контексте написано у Рихтера в CLR via C#.
В этой строке у вас ошибка AppDomain.Unload(testDomain); // выгружается код и очищается вся память (вызывается финализатор), файл удаляется
В этот момент никакой финализатор не вызывается, здесь происходит выгрузка из памяти, именно очищение. Вашего объекта в этот момент то уже и нет, объект удалился, когда вы вышли из анонимного метода, где было единственное его упоминание.
В этот момент никакой финализатор не вызывается, здесь происходит выгрузка из памяти, именно очищение. Вашего объекта в этот момент то уже и нет, объект удалился, когда вы вышли из анонимного метода, где было единственное его упоминание.


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


И тем не менее, с большой вероятностью финализатор отработает (в реальном длительном процессе), и следовательно, вполне логично, в финализаторе написать код, который будет удалять внешний ресурс (если Dispose не был вызван).
Мне кажется это очень логичным. И если есть код в финализаторе, то мне хочется иметь для него тест.

Как мне помнится, сборщик мусора один на все, и к тому моменту, как у вас подходит дело к проверкt а удален ли файл, финализатор может вообще еще не отработал, и это не значит, что код сломался.

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

Если вам нужно обязательно удалять файл, тогда требуется использовать патерн Disposable и явным образом вызывать метод Dispose.
use (var file = new TempFile()){
SomeOperations();
}


В простых случаях так и реализовано, но есть ситуации где код более сложен.
Кстати, для нахождения таких ситуаций, когда Dispose() не вызывался, я добавил конструкторе сохранения стека вызова, а в финализаторе сделал вывод этого стека в консоль. Такой способ помог мне найти и частично решить проблемы не вызова Dispose()
А если в конце анонимной функции внутри testDomain.DoCallBack поставить GC.KeepAlive(temporaryFile);?
Не обнаружил никаких изменений в работе теста после добавления KeepAlive.
Sign up to leave a comment.

Articles