Skip to content

Conversation

@vgvassilev
Copy link
Contributor

@mcbarton, @fsfod, do you have a clue why, although gtest is passing, on windows we get:

  2/2 Test #2: cppinterop-DynamicLibraryManagerTests ...   Passed    0.02 sec
  
  100% tests passed, 0 tests failed out of 2
  
  Label Time Summary:
  DEPENDS    =  13.82 sec*proc (2 tests)
  
  Total Test time (real) =  13.94 sec
  Building Custom Rule D:/a/CppInterOp/CppInterOp/unittests/CMakeLists.txt
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'D:\a\CppInterOp\CppInterOp\build\CMakeFiles\f59abe5bb8dc1f8e4095ecae6605a895\check-cppinterop.rule;D:\a\CppInterOp\CppInterOp\unittests\CMakeLists.txt' exited with code -1. [D:\a\CppInterOp\CppInterOp\build\unittests\check-cppinterop.vcxproj]

More here.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

static bool exec(const char* cmd, std::vector<std::string>& outputs) {
#define DEBUG_TYPE "exec"

std::array<char, 256> buffer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 256 is a magic number; consider replacing it with a named constant [readability-magic-numbers]

    std::array<char, 256> buffer;
                     ^


std::string DetectResourceDir(const char* ClangBinaryName /* = clang */) {
std::string cmd = std::string(ClangBinaryName) + " -print-resource-dir";
std::vector<std::string> outs;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'outs' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::vector<std::string> outs;
std::vector<std::string> outs = 0;

cmd += CompilerName;
cmd += " -xc++ -E -v /dev/null 2>&1 | sed -n -e '/^.include/,${' -e '/^ "
"\\/.*/p' -e '}'";
std::vector<std::string> outs;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'outs' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::vector<std::string> outs;
std::vector<std::string> outs = 0;

#include "llvm/ADT/SmallString.h"
#include "llvm/Support/Path.h"

#include <gmock/gmock.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'gmock/gmock.h' file not found [clang-diagnostic-error]

#include <gmock/gmock.h>
         ^

#endif // LLVM_BINARY_DIR || !_WIN32
Cpp::CreateInterpreter();
EXPECT_STRNE(Cpp::DetectResourceDir().c_str(), Cpp::GetResourceDir());
llvm::SmallString<256> Clang(LLVM_BINARY_DIR);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 256 is a magic number; consider replacing it with a named constant [readability-magic-numbers]

  llvm::SmallString<256> Clang(LLVM_BINARY_DIR);
                    ^

#else
TEST(InterpreterTest, DetectSystemCompilerIncludePaths) {
#endif // _WIN32
std::vector<std::string> includes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'includes' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::vector<std::string> includes;
std::vector<std::string> includes = 0;

@codecov
Copy link

codecov bot commented Mar 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.74%. Comparing base (a6f001d) to head (1b6cda0).
Report is 91 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #206   +/-   ##
=======================================
  Coverage   78.74%   78.74%           
=======================================
  Files           8        8           
  Lines        3091     3091           
=======================================
  Hits         2434     2434           
  Misses        657      657           

@vgvassilev vgvassilev force-pushed the re-enable-win-tests branch from af529fc to 1b6cda0 Compare March 16, 2024 12:08
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton
Copy link
Collaborator

@vgvassilev I don't know exactly what is causing it to fail . The only thing I can tell from a quick glance is that it appears to have an issue with this line https://github.com/vgvassilev/CppInterOp/blob/1b6cda034fe805f3f9306b35737655631662e6dd/unittests/CMakeLists.txt#L44-L45

@aaronj0 aaronj0 added the tests label May 17, 2024
@mcbarton
Copy link
Collaborator

This PR can be closed since its been solved in #313

@mcbarton mcbarton closed this Oct 27, 2024
@vgvassilev vgvassilev deleted the re-enable-win-tests branch October 27, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants