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.

Sign up for GitHub

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

Jump to bottom

GTSAM_SINGLE_TEST_EXE option causes compile errors #91

Closed
cntaylor opened this issue Jul 22, 2019 · 9 comments · Fixed by #1107
Closed

GTSAM_SINGLE_TEST_EXE option causes compile errors #91

cntaylor opened this issue Jul 22, 2019 · 9 comments · Fixed by #1107
Labels
easy-fix Quick and easy to fix linux Related to Linux windows Related to Windows

Comments

@cntaylor
Copy link
Contributor

Description

If the GTSAM_SINGLE_TEST_EXE option is enabled, then the unit tests fail to compile. For example, check.geometry will not compile under either Linux or Windows. It generates a "multiple definition" error

Steps to reproduce

  1. Turn on the GTSAM_SINGLE_TEST_EXE flag in cmake
  2. Run make check.geometry

Expected behavior

Expect the unit tests to compile without errors.

Environment

This occurs in both Windows and Linux. That said, the GTSAM_SINGLE_TEST_EXE is turned on by default in Windows, but is off by default in Linux. So, this is far more "noticeable" in Windows.

Additional information

Two questions really.

  1. Is there a reason this is turned on by default in Windows? Or a reason for this flag at all?
  2. There are a lot of duplicate tests as you go across the individual programs in "check.geometry" (and lots of other groups of "check" programs). Is there a reason for these duplicates? We can go through and make each test individually names, or we could remove some of the duplicates.
@ProfFan ProfFan added this to the GTSAM 4.1 milestone Sep 23, 2019
@ProfFan
Copy link
Collaborator

ProfFan commented Sep 23, 2019

#121

@ProfFan
Copy link
Collaborator

ProfFan commented Dec 23, 2019

@jlblancoc Do we still have this issue? I think you added AppVeyor CI for Win32 platforms...

@jlblancoc
Copy link
Member

Yes, but it failed for a weird reason that couldn't replicate locally in MSVC. The error message is:

-- Configuring done
-- Generating done
-- Build files have been written to: C:/projects/gtsam/build
The solution file has two projects named "gtsam_examples".

and I got stuck there so...

Apparently, it has nothing to do with the issue in this thread.

@ProfFan
Copy link
Collaborator

ProfFan commented Dec 24, 2019

@jlblancoc I think the problem is with MSBuild. Could you test locally with MSBuild instead of VS?

@jlblancoc
Copy link
Member

@ProfFan : thanks for the pointer. Using msbuild it fails, but invoking cmake --build . it works (however it internally invokes cl!)

@ProfFan
Copy link
Collaborator

ProfFan commented Dec 31, 2019

dotnet/msbuild#3019

@ProfFan
Copy link
Collaborator

ProfFan commented Jul 11, 2020

Is this still an issue?

@varunagrawal varunagrawal removed this from the GTSAM 4.1 milestone Jul 13, 2020
@varunagrawal varunagrawal added the windows Related to Windows label Dec 1, 2020
@varunagrawal
Copy link
Collaborator

This is still an issue. There is name-clashing of global variables and functions happening when we merge various test files.

@varunagrawal varunagrawal added the linux Related to Linux label Dec 1, 2020
@varunagrawal
Copy link
Collaborator

So after some consideration, this issue can be easily solved if we update the namespaces of each of the test files. This is an easy thing to do, but requires a lot of legwork to check and run the tests.

@varunagrawal varunagrawal added the easy-fix Quick and easy to fix label Jan 2, 2021
varunagrawal added a commit that referenced this issue Apr 20, 2021
@varunagrawal
Squashed 'wrap/' changes from 903694b77..b2144a712
048666e 
b2144a712 Merge pull request #95 from borglab/feature/empty-str-default-arg
9f1e727d8 Merge pull request #96 from borglab/fix/cmake
97ee2ff0c fix CMake typo
64a599827 support empty strings as default args
7b14ed542 Merge pull request #94 from borglab/fix/cmake-messages
0978641fe clean up
5b9272557 Merge pull request #91 from borglab/feature/enums
56e6f48b3 Merge pull request #93 from borglab/feature/better-template
27cc7cebf better cmake messages
a6318b567 fix tests
b7f60463f remove export_values()
38304fe0a support for class nested enums
348160740 minor fixes
5b6d66a97 use cpp_class and correct module name
2f7ae0676 add newlines and formatting
6e7cecc50 remove support for enum value assignment
c1dc925a6 formatting
798732598 better pybind template
f6dad2959 pybind_wrapper fixes with formatting
7b4a06560 Merge branch 'master' into feature/enums
1982b7131 more comprehensive tests for enums
3a0eafd66 code for wrapping enums
398780982 tests for enum support

git-subtree-dir: wrap
git-subtree-split: b2144a712953dcc3e001c97c2ace791149c97278
@mikesheffler mikesheffler mentioned this issue Feb 16, 2022
Merged
@dellaert dellaert mentioned this issue Feb 16, 2022
Merged
@dellaert dellaert mentioned this issue Feb 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
easy-fix Quick and easy to fix linux Related to Linux windows Related to Windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix issues with grouped tests borglab/gtsam
4 participants
@varunagrawal @ProfFan @jlblancoc @cntaylor