Skip to content

Update to python3 #1084

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

Merged
merged 3 commits into from
Nov 7, 2019
Merged

Update to python3 #1084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1a44ba
COMP: Fix type mismatch ambiguity
hjmjohnson Nov 4, 2019
d76492c
COMP: Remove shadow variable warning
hjmjohnson Nov 4, 2019
50671dd
ENH: Move to requiring python 3
hjmjohnson Nov 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/jsontestrunner/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
find_package(PythonInterp 2.6)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.12.0)
# The new Python3 module is much more robust than the previous PythonInterp
find_package (Python3 COMPONENTS Interpreter)
# Set variables for backwards compatibility with cmake < 3.12.0
set(PYTHONINTERP_FOUND ${Python3_Interpreter_FOUND})
set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
else()
set(Python_ADDITIONAL_VERSIONS 3.8)
find_package(PythonInterp 3)
endif()

add_executable(jsontestrunner_exe
main.cpp
Expand Down
8 changes: 4 additions & 4 deletions src/test_lib_json/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, arrayInsertAtRandomIndex) {
JSONTEST_ASSERT_EQUAL(Json::Value("index1"), array[2]);
JSONTEST_ASSERT_EQUAL(Json::Value("index2"), array[3]);
// checking address
for (int i = 0; i < 3; i++) {
for (Json::ArrayIndex i = 0; i < 3; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, but I'm kind of curious why this error just started happening. Any idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New apple clang compiler with Mac 10.15, XCode 11.2 refused to compile the previous code due to ambiguities.

JSONTEST_ASSERT_EQUAL(vec[i], &array[i]);
}
vec.push_back(&array[3]);
Expand All @@ -362,7 +362,7 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, arrayInsertAtRandomIndex) {
JSONTEST_ASSERT_EQUAL(Json::Value("index1"), array[3]);
JSONTEST_ASSERT_EQUAL(Json::Value("index2"), array[4]);
// checking address
for (int i = 0; i < 4; i++) {
for (Json::ArrayIndex i = 0; i < 4; i++) {
JSONTEST_ASSERT_EQUAL(vec[i], &array[i]);
}
vec.push_back(&array[4]);
Expand All @@ -376,7 +376,7 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, arrayInsertAtRandomIndex) {
JSONTEST_ASSERT_EQUAL(Json::Value("index2"), array[4]);
JSONTEST_ASSERT_EQUAL(Json::Value("index5"), array[5]);
// checking address
for (int i = 0; i < 5; i++) {
for (Json::ArrayIndex i = 0; i < 5; i++) {
JSONTEST_ASSERT_EQUAL(vec[i], &array[i]);
}
vec.push_back(&array[5]);
Expand Down Expand Up @@ -2234,7 +2234,6 @@ JSONTEST_FIXTURE_LOCAL(StyledStreamWriterTest, writeNestedObjects) {
}

JSONTEST_FIXTURE_LOCAL(StyledStreamWriterTest, multiLineArray) {
Json::StyledStreamWriter writer;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line 2237 shadows line 2261 below, but is not used unit line 2270.

{
// Array member has more than 20 print effect rendering lines
const Json::String expected("[\n\t0,"
Expand Down Expand Up @@ -2268,6 +2267,7 @@ JSONTEST_FIXTURE_LOCAL(StyledStreamWriterTest, multiLineArray) {
JSONTEST_ASSERT_STRING_EQUAL(expected, result);
}
{
Json::StyledStreamWriter writer;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand how adding a writer would remove a shadowed variable. I'm sure it's fine, but could you explain it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the original code the line 2237 shadows line 2261 below it, but is not used unit line 2270.

Scope the variable called "writer" in the blocks that need them. It may also have been OK to remove line 2261 in the original code, but I was not sure what the intent of making a new write was.

Copy link
Member

Choose a reason for hiding this comment

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

I forgot to remove line 2261 when i wrote this testcase.

// Array members do not exceed 21 print effects to render a single line
const Json::String expected("[ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 ]\n");
Json::Value root;
Expand Down