search

microsoft / cpp_client_telemetry

1DS C++ SDK
Apache License 2.0
85 stars 48 forks source link

remove privacyguard and dataviewer tests from main repo #725

Closed lalitb closed 3 years ago

lalitb commented 3 years ago

They are added to submodule repo as part of this PR : https://github.com/microsoft/cpp_client_telemetry_modules/pull/59

Above mentioned PR needs to be merged first to have all the tests passing.

maxgolov commented 3 years ago

@lalitb - something isn't right:

         c1xx : fatal error C1083: Cannot open source file: 'D:\a\cpp_client_telemetry\cpp_client_telemetry\lib\modules\privacyguard\tests\unittests\PrivacyGuardTests.cpp': No such file or directory [D:\a\cpp_client_telemetry\cpp_client_telemetry\tests\unittests\UnitTests.vcxproj]
         c1xx : fatal error C1083: Cannot open source file: 'D:\a\cpp_client_telemetry\cpp_client_telemetry\lib\modules\dataviewer\tests\unittests\DefaultDataViewerTests.cpp': No such file or directory [D:\a\cpp_client_telemetry\cpp_client_telemetry\tests\unittests\UnitTests.vcxproj]

OK, I think you need to git add lib/modules to update it to latest, where you added these files.

lalitb commented 3 years ago

@lalitb - something isn't right:

         c1xx : fatal error C1083: Cannot open source file: 'D:\a\cpp_client_telemetry\cpp_client_telemetry\lib\modules\privacyguard\tests\unittests\PrivacyGuardTests.cpp': No such file or directory [D:\a\cpp_client_telemetry\cpp_client_telemetry\tests\unittests\UnitTests.vcxproj]
         c1xx : fatal error C1083: Cannot open source file: 'D:\a\cpp_client_telemetry\cpp_client_telemetry\lib\modules\dataviewer\tests\unittests\DefaultDataViewerTests.cpp': No such file or directory [D:\a\cpp_client_telemetry\cpp_client_telemetry\tests\unittests\UnitTests.vcxproj]

OK, I think you need to git add lib/modules to update it to latest, where you added these files.

Oh I thought it's because other PR ( in modules repo ) is not yet merged. I am re-running the tests again, as the other PR is merged.

maxgolov commented 3 years ago

Oh I thought it's because other PR ( in modules repo ) is not yet merged. I am re-running the tests again, as the other PR is merged.

Sorry, please pull - I updated your branch with the other PR merged and it is still failing.

lalitb commented 3 years ago

Oh I thought it's because other PR ( in modules repo ) is not yet merged. I am re-running the tests again, as the other PR is merged.

Sorry, please pull - I updated your branch with the other PR merged and it is still failing.

OK, I think this PR should resolve the issue : https://github.com/microsoft/cpp_client_telemetry_modules/pull/60 Please approve it once you have time.

lalitb commented 3 years ago

moving to draft to fix the issues

maxgolov commented 3 years ago

I refreshed your branch again with the updated modules PR.

maxgolov commented 3 years ago

Fixed the typo too :)

maxgolov commented 3 years ago

Appears like we might be having some unrelated iOS break, this could because of Xcode emitting an optimization warning that is treated as error... Let's see if it passes.

sid-dahiya commented 3 years ago

Appears like we might be having some unrelated iOS break, this could because of Xcode emitting an optimization warning that is treated as error... Let's see if it passes.

Seems like a legit failure on iOS:

/Users/runner/work/cpp_client_telemetry/cpp_client_telemetry/lib/modules/dataviewer/tests/unittests/DefaultDataViewerTests.cpp:6:10: fatal error: 'CheckForExceptionOrAbort.hpp' file not found

include "CheckForExceptionOrAbort.hpp"

^~~~~~~~~~ 1 error generated.

maxgolov commented 3 years ago

/Users/runner/work/cpp_client_telemetry/cpp_client_telemetry/lib/modules/dataviewer/tests/unittests/DefaultDataViewerTests.cpp:6:10: fatal error: 'CheckForExceptionOrAbort.hpp' file not found

include "CheckForExceptionOrAbort.hpp"

Let me check. I want this sorted out this week :)

maxgolov commented 3 years ago

@lalitb @sid-dahiya - sorry for naïve question - where's CheckForExceptionOrAbort.hpp coming from? 😄 I do not see it used in any other tests.

maxgolov commented 3 years ago

Found it :) [fixed it - fingers crossed]

lalitb commented 3 years ago

Found it :) [fixed it - fingers crossed]

Thanks @maxgolov @sid-dahiya for fixing this. I thought of looking into it during my morning time, but saw it's already fixed now : )

maxgolov commented 3 years ago

@lalitb Let's just change to Ready for review and merge :)