Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Same test runs multiple times when written in a header and included with different unnormalized paths #45

Closed
martin82 opened this issue Dec 3, 2016 · 6 comments

Comments

@martin82
Copy link

martin82 commented Dec 3, 2016

This happens for me due to "bool TestData::operator<(const TestData& other)" differentiating between files "a/../a/b.h" and "a/b.h", thus including a test twice in "testArray".

Normalizing the paths should help I guess.

@onqtam onqtam changed the title Same test runs multiple times Same test runs multiple times when written in a header and included with different unnormalized paths Dec 3, 2016
@onqtam
Copy link
Member

onqtam commented Jun 1, 2018

damn... I just hit this myself! Still not sure when I'll get to this...

@Wetmelon
Copy link

Wetmelon commented Mar 15, 2019

Hah. I just started using doctest yesterday but I managed to do this to myself too. I'm glad I spotted this issue before I did it though...

I have my tests in the header file (bunch o' templates), and one .cpp includes the header via <header.hpp> and another translation unit includes it via "../Libs/header.hpp". The tests got doubled up... granted, it was still really fast :D

@onqtam
Copy link
Member

onqtam commented Mar 15, 2019

For some reason I cannot reproduce it anymore - using cmake 3.10 and VS 2017 or GCC 8.1 on windows. I just tried with GCC 5.1 and still it resolved paths properly. I thought it is compiler-dependent but it might be CMake-version dependent... No idea.

What I tried locally was to include the same header from 2 different .cpp files using different paths (one used an excessive up-and-down ../same_folder/ but the tests from that header file still got registered only once. No matter what I do I get a nicely normalized path from the expansion of __FILE__ at compile time.

I could try to reproduce it if you give me a small .zip file or a few code snippets containing:

  • a simple minimal cmake file
  • a few sources/headers - just enough to illustrate the problem

@Wetmelon
Copy link

Wetmelon commented Mar 15, 2019

I don't use CMake, just boring makefiles, but I was able to force a duplication very easily. .zip attached. Make sure you fix the doctest path in the makefile.

It seems that any combination of unique pathing methods will cause a duplication. I was actually able to get it to triple-up on the tests by putting <header.hpp> in one, "../inc/header.hpp" in another, and "../../docutest_duplicate_example/inc/header.hpp" in a third file. The tests are in header.hpp obviously.

GCC 8.3
Make 4.2.1
Windows 10

doctest_duplicate_example.zip

@onqtam
Copy link
Member

onqtam commented Mar 16, 2019

Thanks for the .zip file - I reproduced it but unfortunately I don't think it's possible for the framework to correctly normalize all paths - "path/to/header.hpp" may mean 2 different things for 2 different .cpp files if they have different include directories.

All that doctest gets is the expansion of __FILE__ and if that isn't properly normalized by the build system or compiler - it cannot reason about paths with 100% certainty and thus should not make impossible decisions - or the risk is skipping tests which is MUCH worse than re-running tests.

Imagine this:

#include "header.hpp"
#include "path/to/header.hpp"

those could be the same header, or maybe not - it all depends on the include paths the build system provides to the compiler - and if doctest gets only header.hpp and path/to/header.hpp it has no way to figure out if they are the same file or not. This won't be common but its possible for someone to name 2 header files identically and give such fucked up include paths for his "convenience".

Also this problem of having the tests registered multiple times could be solved with more rigorous and strict build system conventions. Somehow with my simple attempts using CMake I always got the full normalized paths in the __FILE__ expansion so perhaps there is a silver bullet.

So I'm closing this issue since there is nothing to be done on the side of the framework. If you think of something feel free to add it in case I'm missing something and I'll reopen it.

@onqtam onqtam closed this as completed Mar 16, 2019
@Wetmelon
Copy link

All that doctest gets is the expansion of FILE

Yeah, if that's the spec for doctest, that's fine. I wonder if new versions of cmake are normalizing before including or something similar.

I typically use the tup build system, which keeps track of all files that are part of the build process in a database, so it can tell which file is being referred to regardless of pathing. I should be able to normalize at that level. It would be interesting if Doctest could do something like that, but certainly not necessary.

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

No branches or pull requests

3 participants