From 8d7cde22c31a59d0503230334b1d68e858364c9a Mon Sep 17 00:00:00 2001 From: Signal Linden Date: Tue, 11 Oct 2022 15:10:04 -0700 Subject: Replace llbase with llsd module --- indra/llcommon/tests/llleap_test.cpp | 2 +- indra/llcommon/tests/llsdserialize_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 7ee36a9ea6..60005fc6a9 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -109,7 +109,7 @@ namespace tut "import os\n" "import sys\n" "\n" - "from llbase import llsd\n" + "import llsd\n" "\n" "class ProtocolError(Exception):\n" " def __init__(self, msg, data):\n" diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index c246f5ee56..be7ec12094 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -1706,7 +1706,7 @@ namespace tut // scanner. import_llsd("import os.path\n" "import sys\n" - "from llbase import llsd\n") + "import llsd\n") {} ~TestPythonCompatible() {} -- cgit v1.2.3 From 045342ba29aae186e13c711bd4dd84377d4a7e43 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 11:28:41 -0400 Subject: SL-18837: Bump the granularity of WorkQueue timing tests. On a low-powered GitHub Mac runner, the system doesn't wake up as soon as it should, and we get spurious "too late" errors. Try a bigger time increment. --- indra/llcommon/tests/workqueue_test.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index 1d73f7aa0d..7655a7aa1f 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -83,7 +83,11 @@ namespace tut // signal the work item that it can quit; consider LLOneShotCond. LLCond data; auto start = WorkQueue::TimePoint::clock::now(); - auto interval = 100ms; + // 2s seems like a long time to wait, since it directly impacts the + // duration of this test program. Unfortunately GitHub's Mac runners + // are pretty wimpy, and we're getting spurious "too late" errors just + // because the thread doesn't wake up as soon as we want. + auto interval = 2s; queue.postEvery( interval, [&data, count = 0] -- cgit v1.2.3 From ca510f6c299335c8db27c65c10a8553801c06023 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 22:08:26 -0400 Subject: SL-18837: Try giving temp Python scripts a .py extension. On GitHub Windows Actions runners, we're getting permissions errors trying to tell the Python interpreter to run a NamedTempFile script. Try using NamedExtTempFile to give each such script a .py extension. --- indra/llcommon/tests/llleap_test.cpp | 222 ++++++++++++++++---------------- indra/llcommon/tests/llprocess_test.cpp | 4 +- 2 files changed, 113 insertions(+), 113 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 60005fc6a9..99fd073dd2 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -213,9 +213,9 @@ namespace tut void object::test<1>() { set_test_name("multiple LLLeap instances"); - NamedTempFile script("py", - "import time\n" - "time.sleep(1)\n"); + NamedExtTempFile script("py", + "import time\n" + "time.sleep(1)\n"); LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))->getWeak()); @@ -231,10 +231,10 @@ namespace tut void object::test<2>() { set_test_name("stderr to log"); - NamedTempFile script("py", - "import sys\n" - "sys.stderr.write('''Hello from Python!\n" - "note partial line''')\n"); + NamedExtTempFile script("py", + "import sys\n" + "sys.stderr.write('''Hello from Python!\n" + "note partial line''')\n"); StringVec vcommand{ PYTHON, script.getName() }; CaptureLog log(LLError::LEVEL_INFO); waitfor(LLLeap::create(get_test_name(), vcommand)); @@ -246,8 +246,8 @@ namespace tut void object::test<3>() { set_test_name("bad stdout protocol"); - NamedTempFile script("py", - "print('Hello from Python!')\n"); + NamedExtTempFile script("py", + "print('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); @@ -259,10 +259,10 @@ namespace tut void object::test<4>() { set_test_name("leftover stdout"); - NamedTempFile script("py", - "import sys\n" - // note lack of newline - "sys.stdout.write('Hello from Python!')\n"); + NamedExtTempFile script("py", + "import sys\n" + // note lack of newline + "sys.stdout.write('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); @@ -274,9 +274,9 @@ namespace tut void object::test<5>() { set_test_name("bad stdout len prefix"); - NamedTempFile script("py", - "import sys\n" - "sys.stdout.write('5a2:something')\n"); + NamedExtTempFile script("py", + "import sys\n" + "sys.stdout.write('5a2:something')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); @@ -381,16 +381,16 @@ namespace tut set_test_name("round trip"); AckAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "from " << reader_module << " import *\n" - // make a request on our little API - "request(pump='" << api.getName() << "', data={})\n" - // wait for its response - "resp = get()\n" - "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" - " else 'bad: ' + str(resp)\n" - "send(pump='" << result.getName() << "', data=result)\n"); + NamedExtTempFile script("py", + boost::phoenix::placeholders::arg1 << + "from " << reader_module << " import *\n" + // make a request on our little API + "request(pump='" << api.getName() << "', data={})\n" + // wait for its response + "resp = get()\n" + "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" + " else 'bad: ' + str(resp)\n" + "send(pump='" << result.getName() << "', data=result)\n"); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); result.ensure(); } @@ -419,37 +419,37 @@ namespace tut // iterations etc. in OS pipes and the LLLeap/LLProcess implementation. ReqIDAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "import sys\n" - "from " << reader_module << " import *\n" - // Note that since reader imports llsd, this - // 'import *' gets us llsd too. - "sample = llsd.format_notation(dict(pump='" << - api.getName() << "', data=dict(reqid=999999, reply=replypump())))\n" - // The whole packet has length prefix too: "len:data" - "samplen = len(str(len(sample))) + 1 + len(sample)\n" - // guess how many messages it will take to - // accumulate BUFFERED_LENGTH - "count = int(" << BUFFERED_LENGTH << "/samplen)\n" - "print('Sending %s requests' % count, file=sys.stderr)\n" - "for i in range(count):\n" - " request('" << api.getName() << "', dict(reqid=i))\n" - // The assumption in this specific test that - // replies will arrive in the same order as - // requests is ONLY valid because the API we're - // invoking sends replies instantly. If the API - // had to wait for some external event before - // sending its reply, replies could arrive in - // arbitrary order, and we'd have to tick them - // off from a set. - "result = ''\n" - "for i in range(count):\n" - " resp = get()\n" - " if resp['data']['reqid'] != i:\n" - " result = 'expected reqid=%s in %s' % (i, resp)\n" - " break\n" - "send(pump='" << result.getName() << "', data=result)\n"); + NamedExtTempFile script("py", + boost::phoenix::placeholders::arg1 << + "import sys\n" + "from " << reader_module << " import *\n" + // Note that since reader imports llsd, this + // 'import *' gets us llsd too. + "sample = llsd.format_notation(dict(pump='" << + api.getName() << "', data=dict(reqid=999999, reply=replypump())))\n" + // The whole packet has length prefix too: "len:data" + "samplen = len(str(len(sample))) + 1 + len(sample)\n" + // guess how many messages it will take to + // accumulate BUFFERED_LENGTH + "count = int(" << BUFFERED_LENGTH << "/samplen)\n" + "print('Sending %s requests' % count, file=sys.stderr)\n" + "for i in range(count):\n" + " request('" << api.getName() << "', dict(reqid=i))\n" + // The assumption in this specific test that + // replies will arrive in the same order as + // requests is ONLY valid because the API we're + // invoking sends replies instantly. If the API + // had to wait for some external event before + // sending its reply, replies could arrive in + // arbitrary order, and we'd have to tick them + // off from a set. + "result = ''\n" + "for i in range(count):\n" + " resp = get()\n" + " if resp['data']['reqid'] != i:\n" + " result = 'expected reqid=%s in %s' % (i, resp)\n" + " break\n" + "send(pump='" << result.getName() << "', data=result)\n"); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), 300); // needs more realtime than most tests result.ensure(); @@ -462,60 +462,60 @@ namespace tut { ReqIDAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "import sys\n" - "from " << reader_module << " import *\n" - // Generate a very large string value. - "desired = int(sys.argv[1])\n" - // 7 chars per item: 6 digits, 1 comma - "count = int((desired - 50)/7)\n" - "large = ''.join('%06d,' % i for i in range(count))\n" - // Pass 'large' as reqid because we know the API - // will echo reqid, and we want to receive it back. - "request('" << api.getName() << "', dict(reqid=large))\n" - "try:\n" - " resp = get()\n" - "except ParseError as e:\n" - " # try to find where e.data diverges from expectation\n" - // Normally we'd expect a 'pump' key in there, - // too, with value replypump(). But Python - // serializes keys in a different order than C++, - // so incoming data start with 'data'. - // Truthfully, though, if we get as far as 'pump' - // before we find a difference, something's very - // strange. - " expect = llsd.format_notation(dict(data=dict(reqid=large)))\n" - " chunk = 40\n" - " for offset in range(0, max(len(e.data), len(expect)), chunk):\n" - " if e.data[offset:offset+chunk] != \\\n" - " expect[offset:offset+chunk]:\n" - " print('Offset %06d: expect %r,\\n'\\\n" - " ' get %r' %\\\n" - " (offset,\n" - " expect[offset:offset+chunk],\n" - " e.data[offset:offset+chunk]),\n" - " file=sys.stderr)\n" - " break\n" - " else:\n" - " print('incoming data matches expect?!', file=sys.stderr)\n" - " send('" << result.getName() << "', '%s: %s' % (e.__class__.__name__, e))\n" - " sys.exit(1)\n" - "\n" - "echoed = resp['data']['reqid']\n" - "if echoed == large:\n" - " send('" << result.getName() << "', '')\n" - " sys.exit(0)\n" - // Here we know echoed did NOT match; try to find where - "for i in range(count):\n" - " start = 7*i\n" - " end = 7*(i+1)\n" - " if end > len(echoed)\\\n" - " or echoed[start:end] != large[start:end]:\n" - " send('" << result.getName() << "',\n" - " 'at offset %s, expected %r but got %r' %\n" - " (start, large[start:end], echoed[start:end]))\n" - "sys.exit(1)\n"); + NamedExtTempFile script("py", + boost::phoenix::placeholders::arg1 << + "import sys\n" + "from " << reader_module << " import *\n" + // Generate a very large string value. + "desired = int(sys.argv[1])\n" + // 7 chars per item: 6 digits, 1 comma + "count = int((desired - 50)/7)\n" + "large = ''.join('%06d,' % i for i in range(count))\n" + // Pass 'large' as reqid because we know the API + // will echo reqid, and we want to receive it back. + "request('" << api.getName() << "', dict(reqid=large))\n" + "try:\n" + " resp = get()\n" + "except ParseError as e:\n" + " # try to find where e.data diverges from expectation\n" + // Normally we'd expect a 'pump' key in there, + // too, with value replypump(). But Python + // serializes keys in a different order than C++, + // so incoming data start with 'data'. + // Truthfully, though, if we get as far as 'pump' + // before we find a difference, something's very + // strange. + " expect = llsd.format_notation(dict(data=dict(reqid=large)))\n" + " chunk = 40\n" + " for offset in range(0, max(len(e.data), len(expect)), chunk):\n" + " if e.data[offset:offset+chunk] != \\\n" + " expect[offset:offset+chunk]:\n" + " print('Offset %06d: expect %r,\\n'\\\n" + " ' get %r' %\\\n" + " (offset,\n" + " expect[offset:offset+chunk],\n" + " e.data[offset:offset+chunk]),\n" + " file=sys.stderr)\n" + " break\n" + " else:\n" + " print('incoming data matches expect?!', file=sys.stderr)\n" + " send('" << result.getName() << "', '%s: %s' % (e.__class__.__name__, e))\n" + " sys.exit(1)\n" + "\n" + "echoed = resp['data']['reqid']\n" + "if echoed == large:\n" + " send('" << result.getName() << "', '')\n" + " sys.exit(0)\n" + // Here we know echoed did NOT match; try to find where + "for i in range(count):\n" + " start = 7*i\n" + " end = 7*(i+1)\n" + " if end > len(echoed)\\\n" + " or echoed[start:end] != large[start:end]:\n" + " send('" << result.getName() << "',\n" + " 'at offset %s, expected %r but got %r' %\n" + " (start, large[start:end], echoed[start:end]))\n" + "sys.exit(1)\n"); waitfor(LLLeap::create(test_name, sv(list_of (PYTHON) diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 81449b4a42..4adb8d872a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -191,7 +191,7 @@ struct PythonProcessLauncher LLProcess::Params mParams; LLProcessPtr mPy; std::string mDesc; - NamedTempFile mScript; + NamedExtTempFile mScript; }; /// convenience function for PythonProcessLauncher::run() @@ -355,7 +355,7 @@ namespace tut set_test_name("raw APR nonblocking I/O"); // Create a script file in a temporary place. - NamedTempFile script("py", + NamedExtTempFile script("py", "from __future__ import print_function" EOL "import sys" EOL "import time" EOL -- cgit v1.2.3 From 6516c9d07db42beba5ba9c0c41a33925794a249c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 07:44:42 -0400 Subject: SL-18837: NamedTempFile back to std::function, use boost::phoenix << It seems the problem addressed by aab769e wasn't some synergy between Boost.Phoenix and Boost.Function, but rather the lack of a Phoenix header file introducing operator<<(). --- indra/llcommon/tests/llleap_test.cpp | 1 + indra/llcommon/tests/llsdserialize_test.cpp | 1 + 2 files changed, 2 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 99fd073dd2..0c91db3e24 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -19,6 +19,7 @@ // external library headers #include #include +#include // operator<<() // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index d7c11c5021..a0b8519508 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -50,6 +50,7 @@ typedef U32 uint32_t; #include "boost/bind.hpp" #include "boost/phoenix/bind/bind_function.hpp" #include "boost/phoenix/core/argument.hpp" +#include using namespace boost::phoenix; #include "../llsd.h" -- cgit v1.2.3 From c4366378b61cacb9d87eb2917302fa17e9207491 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 10:04:26 -0400 Subject: SL-18837: Ditch Boost.Phoenix implicit lambda syntax. It's cool to be able to write 'arg1 << "stuff" << var ...;' for a lambda accepting a std::ostream reference, but cascading compile errors mean it's no longer worth trying to make that work -- given actual C++ lambdas. Also clean up a lingering BOOST_FOREACH() and a boost::bind() while at it. --- indra/llcommon/tests/llleap_test.cpp | 18 ++++++++---------- indra/llcommon/tests/llsdserialize_test.cpp | 21 +++++++-------------- 2 files changed, 15 insertions(+), 24 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 0c91db3e24..0fc741d9e1 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -18,8 +18,6 @@ #include // external library headers #include -#include -#include // operator<<() // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -105,7 +103,7 @@ namespace tut llleap_data(): reader(".py", // This logic is adapted from vita.viewerclient.receiveEvent() - boost::phoenix::placeholders::arg1 << + [](std::ostream& out){ out << "import re\n" "import os\n" "import sys\n" @@ -189,7 +187,7 @@ namespace tut "def request(pump, data):\n" " # we expect 'data' is a dict\n" " data['reply'] = _reply\n" - " send(pump, data)\n"), + " send(pump, data)\n";}), // Get the actual pathname of the NamedExtTempFile and trim off // the ".py" extension. (We could cache reader.getName() in a // separate member variable, but I happen to know getName() just @@ -383,7 +381,7 @@ namespace tut AckAPI api; Result result; NamedExtTempFile script("py", - boost::phoenix::placeholders::arg1 << + [&](std::ostream& out){ out << "from " << reader_module << " import *\n" // make a request on our little API "request(pump='" << api.getName() << "', data={})\n" @@ -391,7 +389,7 @@ namespace tut "resp = get()\n" "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" " else 'bad: ' + str(resp)\n" - "send(pump='" << result.getName() << "', data=result)\n"); + "send(pump='" << result.getName() << "', data=result)\n";}); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); result.ensure(); } @@ -421,7 +419,7 @@ namespace tut ReqIDAPI api; Result result; NamedExtTempFile script("py", - boost::phoenix::placeholders::arg1 << + [&](std::ostream& out){ out << "import sys\n" "from " << reader_module << " import *\n" // Note that since reader imports llsd, this @@ -450,7 +448,7 @@ namespace tut " if resp['data']['reqid'] != i:\n" " result = 'expected reqid=%s in %s' % (i, resp)\n" " break\n" - "send(pump='" << result.getName() << "', data=result)\n"); + "send(pump='" << result.getName() << "', data=result)\n";}); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), 300); // needs more realtime than most tests result.ensure(); @@ -464,7 +462,7 @@ namespace tut ReqIDAPI api; Result result; NamedExtTempFile script("py", - boost::phoenix::placeholders::arg1 << + [&](std::ostream& out){ out << "import sys\n" "from " << reader_module << " import *\n" // Generate a very large string value. @@ -516,7 +514,7 @@ namespace tut " send('" << result.getName() << "',\n" " 'at offset %s, expected %r but got %r' %\n" " (start, large[start:end], echoed[start:end]))\n" - "sys.exit(1)\n"); + "sys.exit(1)\n";}); waitfor(LLLeap::create(test_name, sv(list_of (PYTHON) diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index a0b8519508..08bf7fbc51 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -45,13 +45,6 @@ typedef U32 uint32_t; #endif #include "boost/range.hpp" -#include "boost/foreach.hpp" -#include "boost/function.hpp" -#include "boost/bind.hpp" -#include "boost/phoenix/bind/bind_function.hpp" -#include "boost/phoenix/core/argument.hpp" -#include -using namespace boost::phoenix; #include "../llsd.h" #include "../llsdserialize.h" @@ -1802,7 +1795,7 @@ namespace tut // helper for test<3> static void writeLLSDArray(std::ostream& out, const LLSD& array) { - BOOST_FOREACH(LLSD item, llsd::inArray(array)) + for (const LLSD& item: llsd::inArray(array)) { LLSDSerialize::toNotation(item, out); // It's important to separate with newlines because Python's llsd @@ -1842,21 +1835,21 @@ namespace tut // Create an llsdXXXXXX file containing 'data' serialized to // notation. NamedTempFile file("llsd", - // NamedTempFile's boost::function constructor + // NamedTempFile's function constructor // takes a callable. To this callable it passes the // std::ostream with which it's writing the // NamedTempFile. - boost::bind(writeLLSDArray, _1, cdata)); + [&](std::ostream& out){ writeLLSDArray(out, cdata); }); python("read C++ notation", - placeholders::arg1 << + [&](std::ostream& out){ out << import_llsd << "def parse_each(iterable):\n" " for item in iterable:\n" " yield llsd.parse(item)\n" << pydata << // Don't forget raw-string syntax for Windows pathnames. - "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n"); + "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n";}); } template<> template<> @@ -1870,7 +1863,7 @@ namespace tut NamedTempFile file("llsd", ""); python("write Python notation", - placeholders::arg1 << + [&](std::ostream& out){ out << import_llsd << "DATA = [\n" " 17,\n" @@ -1884,7 +1877,7 @@ namespace tut // N.B. Using 'print' implicitly adds newlines. "with open(r'" << file.getName() << "', 'w') as f:\n" " for item in DATA:\n" - " print(llsd.format_notation(item).decode(), file=f)\n"); + " print(llsd.format_notation(item).decode(), file=f)\n";}); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From e933ace53b24b732d4111169e3c5964a8591a29e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 14:07:12 -0400 Subject: SL-18837: Try to bypass Windows perm problem with Python indirection. --- indra/llcommon/tests/llprocess_test.cpp | 41 ++++++++++++++++++------------ indra/llcommon/tests/test_python_script.py | 20 +++++++++++++++ 2 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 indra/llcommon/tests/test_python_script.py (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 4adb8d872a..af99e97d66 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -124,6 +124,17 @@ void waitfor(LLProcess::handle h, const std::string& desc, int timeout=60) i < timeout); } +namespace { + +// find test helper, a sibling of this file +// nat 2023-07-07: we're currently using Boost 1.81, but +// path::replace_filename() (which is exactly what we need here) doesn't +// arrive until Boost 1.82. +auto test_python_script{ + (boost::filesystem::path(__FILE__).remove_filename() / "test_python_script.py").string() }; + +} + /** * Construct an LLProcess to run a Python script. */ @@ -145,6 +156,7 @@ struct PythonProcessLauncher mParams.desc = desc + " script"; mParams.executable = PYTHON; + mParams.args.add(test_python_script); mParams.args.add(mScript.getName()); } @@ -214,30 +226,26 @@ static std::string python_out(const std::string& desc, const CONTENT& script) class NamedTempDir: public boost::noncopyable { public: - // Use python() function to create a temp directory: I've found - // nothing in either Boost.Filesystem or APR quite like Python's - // tempfile.mkdtemp(). - // Special extra bonus: on Mac, mkdtemp() reports a pathname - // starting with /var/folders/something, whereas that's really a - // symlink to /private/var/folders/something. Have to use - // realpath() to compare properly. NamedTempDir(): - mPath(python_out("mkdtemp()", - "from __future__ import with_statement\n" - "import os.path, sys, tempfile\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.path.normcase(os.path.normpath(os.path.realpath(tempfile.mkdtemp()))))\n")) - {} + mPath(NamedTempFile::temp_path()), + mCreated(boost::filesystem::create_directories(mPath)) + { + mPath = boost::filesystem::canonical(mPath); + } ~NamedTempDir() { - aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); + if (mCreated) + { + boost::filesystem::remove_all(mPath); + } } - std::string getName() const { return mPath; } + std::string getName() const { return mPath.string(); } private: - std::string mPath; + boost::filesystem::path mPath; + bool mCreated; }; /***************************************************************************** @@ -390,6 +398,7 @@ namespace tut // Have to have a named copy of this std::string so its c_str() value // will persist. std::string scriptname(script.getName()); + argv.push_back(test_python_script.c_str()); argv.push_back(scriptname.c_str()); argv.push_back(NULL); diff --git a/indra/llcommon/tests/test_python_script.py b/indra/llcommon/tests/test_python_script.py new file mode 100644 index 0000000000..c0c8661aa9 --- /dev/null +++ b/indra/llcommon/tests/test_python_script.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +"""\ +@file test_python_script.py +@author Nat Goodspeed +@date 2023-07-07 +@brief Work around a problem running Python within integration tests on GitHub + Windows runners. + +$LicenseInfo:firstyear=2023&license=viewerlgpl$ +Copyright (c) 2023, Linden Research, Inc. +$/LicenseInfo$ +""" + +import os +import sys + +# use pop() so that if the referenced script in turn looks at sys.argv, it +# will see its arguments rather than its own filename +_script = sys.argv.pop(1) +exec(open(_script).read()) -- cgit v1.2.3 From c4b5d089dad5680a0dd12b2d386b692318eb5c58 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 16:57:20 -0400 Subject: SL-18837: Partially revert e933ace, keeping useful tweaks. Introducing indirection via test_python_script.py did NOT address the "Access is denied" errors on GitHub Windows runners. --- indra/llcommon/tests/llleap_test.cpp | 25 +++++++++---------------- indra/llcommon/tests/llprocess_test.cpp | 13 ------------- indra/llcommon/tests/test_python_script.py | 20 -------------------- 3 files changed, 9 insertions(+), 49 deletions(-) delete mode 100644 indra/llcommon/tests/test_python_script.py (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 0fc741d9e1..e9edd165df 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -17,7 +17,6 @@ // std headers #include // external library headers -#include // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -29,10 +28,6 @@ #include "stringize.h" #include "StringVec.h" -using boost::assign::list_of; - -StringVec sv(const StringVec& listof) { return listof; } - #if defined(LL_WINDOWS) #define sleep(secs) _sleep((secs) * 1000) @@ -217,9 +212,9 @@ namespace tut "time.sleep(1)\n"); LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))->getWeak()); + StringVec{PYTHON, script.getName()})->getWeak()); instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))->getWeak()); + StringVec{PYTHON, script.getName()})->getWeak()); // In this case we're simply establishing that two LLLeap instances // can coexist without throwing exceptions or bombing in any other // way. Wait for them to terminate. @@ -249,7 +244,7 @@ namespace tut "print('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("invalid protocol"), "Hello from Python!"); } @@ -264,7 +259,7 @@ namespace tut "sys.stdout.write('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("Discarding"), "Hello from Python!"); } @@ -278,7 +273,7 @@ namespace tut "sys.stdout.write('5a2:something')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("invalid protocol"), "5a2:"); } @@ -390,7 +385,8 @@ namespace tut "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" " else 'bad: ' + str(resp)\n" "send(pump='" << result.getName() << "', data=result)\n";}); - waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); + waitfor(LLLeap::create(get_test_name(), + StringVec{PYTHON, script.getName()})); result.ensure(); } @@ -449,7 +445,7 @@ namespace tut " result = 'expected reqid=%s in %s' % (i, resp)\n" " break\n" "send(pump='" << result.getName() << "', data=result)\n";}); - waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), + waitfor(LLLeap::create(get_test_name(), StringVec{PYTHON, script.getName()}), 300); // needs more realtime than most tests result.ensure(); } @@ -516,10 +512,7 @@ namespace tut " (start, large[start:end], echoed[start:end]))\n" "sys.exit(1)\n";}); waitfor(LLLeap::create(test_name, - sv(list_of - (PYTHON) - (script.getName()) - (stringize(size)))), + StringVec{PYTHON, script.getName(), stringize(size)}), 180); // try a longer timeout result.ensure(); } diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index af99e97d66..c7d1a2c86a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -124,17 +124,6 @@ void waitfor(LLProcess::handle h, const std::string& desc, int timeout=60) i < timeout); } -namespace { - -// find test helper, a sibling of this file -// nat 2023-07-07: we're currently using Boost 1.81, but -// path::replace_filename() (which is exactly what we need here) doesn't -// arrive until Boost 1.82. -auto test_python_script{ - (boost::filesystem::path(__FILE__).remove_filename() / "test_python_script.py").string() }; - -} - /** * Construct an LLProcess to run a Python script. */ @@ -156,7 +145,6 @@ struct PythonProcessLauncher mParams.desc = desc + " script"; mParams.executable = PYTHON; - mParams.args.add(test_python_script); mParams.args.add(mScript.getName()); } @@ -398,7 +386,6 @@ namespace tut // Have to have a named copy of this std::string so its c_str() value // will persist. std::string scriptname(script.getName()); - argv.push_back(test_python_script.c_str()); argv.push_back(scriptname.c_str()); argv.push_back(NULL); diff --git a/indra/llcommon/tests/test_python_script.py b/indra/llcommon/tests/test_python_script.py deleted file mode 100644 index c0c8661aa9..0000000000 --- a/indra/llcommon/tests/test_python_script.py +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env python3 -"""\ -@file test_python_script.py -@author Nat Goodspeed -@date 2023-07-07 -@brief Work around a problem running Python within integration tests on GitHub - Windows runners. - -$LicenseInfo:firstyear=2023&license=viewerlgpl$ -Copyright (c) 2023, Linden Research, Inc. -$/LicenseInfo$ -""" - -import os -import sys - -# use pop() so that if the referenced script in turn looks at sys.argv, it -# will see its arguments rather than its own filename -_script = sys.argv.pop(1) -exec(open(_script).read()) -- cgit v1.2.3 From 1fc8758458c99b3a41965e33b3c62613c83e403a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 17:31:50 -0400 Subject: SL-18837: Coax APR to log LLProcess launch attempts; show log file. --- indra/llcommon/tests/llprocess_test.cpp | 34 +++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c7d1a2c86a..fbddc7f909 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -30,6 +30,7 @@ #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" #include "stringize.h" +#include "lldir.h" #include "llsdutil.h" #include "llevents.h" #include "llstring.h" @@ -151,8 +152,37 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - mPy = LLProcess::create(mParams); - tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); + std::string logpath{ gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "apr.log") }; +#if LL_WINDOWS + _putenv_s("APR_LOG", logpath.c_str()); +#else + setenv("APR_LOG", logpath.c_str(), 1); +#endif + try + { + mPy = LLProcess::create(mParams); + tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); + } + catch (const tut::failure& err); + { + std::ifstream inf(logpath.c_str()); + if (! inf.is_open()) + { + LL_WARNS() << "Couldn't open '" << logpath << "'" << LL_ENDL; + } + else + { + LL_WARNS() << "==============================" << LL_ENDL; + LL_WARNS() << "From '" << logpath << "':" << LL_ENDL; + std::string line; + while (std::getline(line, inf)) + { + LL_WARNS() << line << LL_ENDL; + } + LL_WARNS() << "==============================" << LL_ENDL; + } + throw; + } } /// Run Python script and wait for it to complete. -- cgit v1.2.3 From 8f81e1fa87123ff6255e9ee82e68c414efe05cdd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 17:47:57 -0400 Subject: SL-18837: Fix "lldir.h" #include --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index fbddc7f909..9ee7890c7c 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -30,7 +30,7 @@ #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" #include "stringize.h" -#include "lldir.h" +#include "../llfilesystem/lldir.h" #include "llsdutil.h" #include "llevents.h" #include "llstring.h" -- cgit v1.2.3 From 8aa3a0a7ed8cf3e3fedb2c98d6ea336fdd45e296 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 19:48:02 -0400 Subject: SL-18837: Fix spurious semi --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 9ee7890c7c..fb5cf12cb2 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -163,7 +163,7 @@ struct PythonProcessLauncher mPy = LLProcess::create(mParams); tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); } - catch (const tut::failure& err); + catch (const tut::failure& err) { std::ifstream inf(logpath.c_str()); if (! inf.is_open()) -- cgit v1.2.3 From 09c5b01997e1d34e799a8a0ee3571bd181f9a665 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 20:02:33 -0400 Subject: SL-18837: Hook in LLDir to allow reading APR log file. --- indra/llcommon/CMakeLists.txt | 2 +- indra/llcommon/tests/llprocess_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 54020a4231..7412514e6b 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -308,7 +308,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llmainthreadtask "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs};llfilesystem") LL_ADD_INTEGRATION_TEST(llprocessor "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocinfo "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}") diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index fb5cf12cb2..c6091bfeb1 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -175,7 +175,7 @@ struct PythonProcessLauncher LL_WARNS() << "==============================" << LL_ENDL; LL_WARNS() << "From '" << logpath << "':" << LL_ENDL; std::string line; - while (std::getline(line, inf)) + while (std::getline(inf, line)) { LL_WARNS() << line << LL_ENDL; } -- cgit v1.2.3 From 908fb3fed6b858da4dc2b1c840b849e30ade2046 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 20:54:34 -0400 Subject: SL-18837: Ditch unreferenced name of caught exception --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c6091bfeb1..827837d62a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -163,7 +163,7 @@ struct PythonProcessLauncher mPy = LLProcess::create(mParams); tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); } - catch (const tut::failure& err) + catch (const tut::failure&) { std::ifstream inf(logpath.c_str()); if (! inf.is_open()) -- cgit v1.2.3 From f37d2c307617302f2ed5dfead7e280da54a7d3e4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 8 Jul 2023 09:04:33 -0400 Subject: SL-18837: Don't use LLDir, use NamedTempFile::temp_path. Remove llcommon circular dependency on llfilesystem, which doesn't work for this case anyway. --- indra/llcommon/CMakeLists.txt | 2 +- indra/llcommon/tests/llprocess_test.cpp | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 7412514e6b..54020a4231 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -308,7 +308,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llmainthreadtask "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs};llfilesystem") + LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocessor "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocinfo "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}") diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 827837d62a..6fcc6fd8aa 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -30,7 +30,6 @@ #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" #include "stringize.h" -#include "../llfilesystem/lldir.h" #include "llsdutil.h" #include "llevents.h" #include "llstring.h" @@ -152,11 +151,9 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - std::string logpath{ gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "apr.log") }; + std::string logpath{ NamedTempFile::temp_path("apr", ".log").string() }; #if LL_WINDOWS _putenv_s("APR_LOG", logpath.c_str()); -#else - setenv("APR_LOG", logpath.c_str(), 1); #endif try { -- cgit v1.2.3 From 1ec6c744048a2905b0f2bf83f035a8fb8798dbdf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 8 Jul 2023 11:08:16 -0400 Subject: SL-18837: Set APR_LOG once for the whole job instead of a new value for each LLProcess::create() invocation. Since the internal apr_log() function only looks at APR_LOG once per process, the first test (which succeeded, hence no log file dump) left the log file open with that same original pathname. Resetting the APR_LOG environment variable for subsequent runs only made the new code in llprocess_test look for files that were never created. --- indra/llcommon/tests/llprocess_test.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 6fcc6fd8aa..a01ec84547 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -151,10 +151,6 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - std::string logpath{ NamedTempFile::temp_path("apr", ".log").string() }; -#if LL_WINDOWS - _putenv_s("APR_LOG", logpath.c_str()); -#endif try { mPy = LLProcess::create(mParams); @@ -162,21 +158,25 @@ struct PythonProcessLauncher } catch (const tut::failure&) { - std::ifstream inf(logpath.c_str()); - if (! inf.is_open()) - { - LL_WARNS() << "Couldn't open '" << logpath << "'" << LL_ENDL; - } - else + const char* APR_LOG = getenv("APR_LOG"); + if (APR_LOG && *APR_LOG) { - LL_WARNS() << "==============================" << LL_ENDL; - LL_WARNS() << "From '" << logpath << "':" << LL_ENDL; - std::string line; - while (std::getline(inf, line)) + std::ifstream inf(APR_LOG); + if (! inf.is_open()) { - LL_WARNS() << line << LL_ENDL; + LL_WARNS() << "Couldn't open '" << APR_LOG << "'" << LL_ENDL; + } + else + { + LL_WARNS() << "==============================" << LL_ENDL; + LL_WARNS() << "From '" << APR_LOG << "':" << LL_ENDL; + std::string line; + while (std::getline(inf, line)) + { + LL_WARNS() << line << LL_ENDL; + } + LL_WARNS() << "==============================" << LL_ENDL; } - LL_WARNS() << "==============================" << LL_ENDL; } throw; } -- cgit v1.2.3 From 7dc6211ad5ea83685a35c6fff740278343aa8b9d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 8 Jul 2023 14:08:16 -0400 Subject: SL-18837: Force llprocess_test and llleap_test to use just 'python'. On GitHub Windows runners, trying to make build.yaml set PYTHON=python in the environment doesn't work: integration tests still fail with "Access is denied" because they're still trying to execute the interpreter's full pathname. Instead, make llprocess_test and llleap_test detect the case of GitHub Windows and override the environment variable PYTHON with a baked-in string constant "python". --- indra/llcommon/tests/llleap_test.cpp | 11 ++++++++++- indra/llcommon/tests/llprocess_test.cpp | 13 ++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index e9edd165df..01515ecebf 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -193,11 +193,20 @@ namespace tut reader.getName().substr(0, reader.getName().length()-3))), PYTHON(LLStringUtil::getenv("PYTHON")) { +#if LL_WINDOWS + // Weirdly, on GitHub Windows runners, plain 'python' works much + // better than a full pathname. + const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); + if (RUNNER_TEMP && *RUNNER_TEMP) + { + PYTHON = "python"; + } +#endif ensure("Set PYTHON to interpreter pathname", !PYTHON.empty()); } NamedExtTempFile reader; const std::string reader_module; - const std::string PYTHON; + std::string PYTHON; }; typedef test_group llleap_group; typedef llleap_group::object object; diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index a01ec84547..3ba3a8aab3 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -141,6 +141,15 @@ struct PythonProcessLauncher mScript("py", script) { auto PYTHON(LLStringUtil::getenv("PYTHON")); +#if LL_WINDOWS + // Weirdly, on GitHub Windows runners, plain 'python' works much better + // than a full pathname. + const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); + if (RUNNER_TEMP && *RUNNER_TEMP) + { + PYTHON = "python"; + } +#endif tut::ensure("Set $PYTHON to the Python interpreter", !PYTHON.empty()); mParams.desc = desc + " script"; @@ -1013,7 +1022,9 @@ namespace tut set_test_name("get*Pipe() validation"); PythonProcessLauncher py(get_test_name(), "from __future__ import print_function\n" - "print('this output is expected')\n"); + "import sys\n" + "print('this output is expected')\n" + "print('run by', sys.executable)\n"); py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stdin py.mParams.files.add(LLProcess::FileParam()); // inherit stdout py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stderr -- cgit v1.2.3 From 31ccef8a666da54312a55663a7ac03061c4903be Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 Jul 2023 14:35:41 -0400 Subject: SL-18837: Revert "Force llprocess_test and llleap_test to use just 'python'." Turns out that the pathname of the Python executable wasn't the issue. This reverts commit 7dc6211ad5ea83685a35c6fff740278343aa8b9d. --- indra/llcommon/tests/llleap_test.cpp | 11 +---------- indra/llcommon/tests/llprocess_test.cpp | 13 +------------ 2 files changed, 2 insertions(+), 22 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 01515ecebf..e9edd165df 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -193,20 +193,11 @@ namespace tut reader.getName().substr(0, reader.getName().length()-3))), PYTHON(LLStringUtil::getenv("PYTHON")) { -#if LL_WINDOWS - // Weirdly, on GitHub Windows runners, plain 'python' works much - // better than a full pathname. - const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); - if (RUNNER_TEMP && *RUNNER_TEMP) - { - PYTHON = "python"; - } -#endif ensure("Set PYTHON to interpreter pathname", !PYTHON.empty()); } NamedExtTempFile reader; const std::string reader_module; - std::string PYTHON; + const std::string PYTHON; }; typedef test_group llleap_group; typedef llleap_group::object object; diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 3ba3a8aab3..a01ec84547 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -141,15 +141,6 @@ struct PythonProcessLauncher mScript("py", script) { auto PYTHON(LLStringUtil::getenv("PYTHON")); -#if LL_WINDOWS - // Weirdly, on GitHub Windows runners, plain 'python' works much better - // than a full pathname. - const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); - if (RUNNER_TEMP && *RUNNER_TEMP) - { - PYTHON = "python"; - } -#endif tut::ensure("Set $PYTHON to the Python interpreter", !PYTHON.empty()); mParams.desc = desc + " script"; @@ -1022,9 +1013,7 @@ namespace tut set_test_name("get*Pipe() validation"); PythonProcessLauncher py(get_test_name(), "from __future__ import print_function\n" - "import sys\n" - "print('this output is expected')\n" - "print('run by', sys.executable)\n"); + "print('this output is expected')\n"); py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stdin py.mParams.files.add(LLProcess::FileParam()); // inherit stdout py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stderr -- cgit v1.2.3 From d8292a629149c2cfdda6ae9df4e87aa117153c21 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 Jul 2023 14:46:14 -0400 Subject: SL-18837: Disable APR_LOG for now, but leave notes for the future. --- indra/llcommon/tests/llprocess_test.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index a01ec84547..9ca664c80c 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -158,6 +158,9 @@ struct PythonProcessLauncher } catch (const tut::failure&) { + // On Windows, if APR_LOG is set, our version of APR's + // apr_create_proc() logs to the specified file. If this test + // failed, try to report that log. const char* APR_LOG = getenv("APR_LOG"); if (APR_LOG && *APR_LOG) { -- cgit v1.2.3 From c77737b925e3687e47d3a1dce1b7e8b481302741 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 Jul 2023 15:26:21 -0400 Subject: SL-18837: Windows failures in setWorkingDirectory(): C: vs. c: (sigh) Normalize the case of the name of the temp directory for string comparison. --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 9ca664c80c..c1cb2af7fe 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -591,7 +591,7 @@ namespace tut " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); - ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); + ensure_equals("os.getcwd()", py.run_read(), utf8str_tolower(tempdir.getName())); } template<> template<> -- cgit v1.2.3 From 54f9ca5404c05a4031c1c12caf24b88048704cbd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 15:41:26 -0400 Subject: SL-18837: Merge branch 'actions' into actions-build-sh --- indra/llcommon/llprocessor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llprocessor.cpp b/indra/llcommon/llprocessor.cpp index 4a1a81f083..28f8bc2b93 100644 --- a/indra/llcommon/llprocessor.cpp +++ b/indra/llcommon/llprocessor.cpp @@ -746,7 +746,7 @@ private: __cpuid(0x1, eax, ebx, ecx, edx); if(feature_infos[0] != (S32)edx) { - LL_ERRS() << "machdep.cpu.feature_bits doesn't match expected cpuid result!" << LL_ENDL; + LL_WARNS() << "machdep.cpu.feature_bits doesn't match expected cpuid result!" << LL_ENDL; } #endif // LL_RELEASE_FOR_DOWNLOAD -- cgit v1.2.3 From 167ac704c8387c531630949860e33fb5a59789a8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 16:20:59 -0400 Subject: SL-18837: Clean up some redundancy in llrand.cpp. --- indra/llcommon/llrand.cpp | 96 +++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 41 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index cb28a8f5c3..4e4345f37a 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,14 +58,18 @@ * to restore uniform distribution. */ +template +inline REAL ll_internal_random(); + // *NOTE: The system rand implementation is probably not correct. #define LL_USE_SYSTEM_RAND 0 +/***************************************************************************** +* The LL_USE_SYSTEM_RAND implementation has been disabled since June 2008. +*****************************************************************************/ #if LL_USE_SYSTEM_RAND #include -#endif -#if LL_USE_SYSTEM_RAND class LLSeedRand { public: @@ -79,25 +83,25 @@ public: } }; static LLSeedRand sRandomSeeder; -inline F64 ll_internal_random_double() + +template <> +inline F64 ll_internal_random() { #if LL_WINDOWS - return (F64)rand() / (F64)RAND_MAX; + return (F64)rand() / (F64)(RAND_MAX+1); #else return drand48(); #endif } -inline F32 ll_internal_random_float() -{ -#if LL_WINDOWS - return (F32)rand() / (F32)RAND_MAX; -#else - return (F32)drand48(); -#endif -} -#else + +/***************************************************************************** +* This is the implementation we've been using. +*****************************************************************************/ +#else // LL_USE_SYSTEM_RAND static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); -inline F64 ll_internal_random_double() + +template <> +inline F64 ll_internal_random() { // *HACK: Through experimentation, we have found that dual core // CPUs (or at least multi-threaded processes) seem to @@ -107,16 +111,40 @@ inline F64 ll_internal_random_double() if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } +#endif // LL_USE_SYSTEM_RAND + +/***************************************************************************** +* Functions common to both implementations +*****************************************************************************/ +template <> +inline F32 ll_internal_random() +{ + return F32(ll_internal_random()); +} + +/*------------------------------ F64 aliases -------------------------------*/ +inline F64 ll_internal_random_double() +{ + return ll_internal_random(); +} + +F64 ll_drand() +{ + return ll_internal_random_double(); +} +/*------------------------------ F32 aliases -------------------------------*/ inline F32 ll_internal_random_float() { - // The clamping rules are described above. - F32 rv = (F32)gRandomGenerator(); - if(!((rv >= 0.0f) && (rv < 1.0f))) return fmod(rv, 1.f); - return rv; + return ll_internal_random(); } -#endif +F32 ll_frand() +{ + return ll_internal_random_float(); +} + +/*-------------------------- clamped random range --------------------------*/ S32 ll_rand() { return ll_rand(RAND_MAX); @@ -130,42 +158,28 @@ S32 ll_rand(S32 val) return rv; } -F32 ll_frand() -{ - return ll_internal_random_float(); -} - -F32 ll_frand(F32 val) +template +REAL ll_grand(REAL val) { // The clamping rules are described above. - F32 rv = ll_internal_random_float() * val; + REAL rv = ll_internal_random() * val; if(val > 0) { - if(rv >= val) return 0.0f; + if(rv >= val) return REAL(); } else { - if(rv <= val) return 0.0f; + if(rv <= val) return REAL(); } return rv; } -F64 ll_drand() +F32 ll_frand(F32 val) { - return ll_internal_random_double(); + return ll_grand(val); } F64 ll_drand(F64 val) { - // The clamping rules are described above. - F64 rv = ll_internal_random_double() * val; - if(val > 0) - { - if(rv >= val) return 0.0; - } - else - { - if(rv <= val) return 0.0; - } - return rv; + return ll_grand(val); } -- cgit v1.2.3 From 4b158580e5654615d2a5510267bf76392c9666fa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 16:47:50 -0400 Subject: SL-18837: Lowercasing pathname for string compare is Windows-only. --- indra/llcommon/tests/llprocess_test.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c1cb2af7fe..b6b297b8d7 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -591,7 +591,13 @@ namespace tut " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); - ensure_equals("os.getcwd()", py.run_read(), utf8str_tolower(tempdir.getName())); + std::string expected{ tempdir.getName() }; +#if LL_WINDOWS + // SIGH, don't get tripped up by "C:" != "c:" -- + // but on the Mac, using tolower() fails because "/users" != "/Users"! + expected = utf8str_tolower(expected); +#endif + ensure_equals("os.getcwd()", py.run_read(), expected); } template<> template<> -- cgit v1.2.3 From 14d0b514af4e70956ecc2fbf6fe4c2e745e144a8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 Jul 2023 09:45:00 -0400 Subject: SL-18837: Ditch inactive llrand.cpp LL_USE_SYSTEM_RAND code. LL_USE_SYSTEM_RAND has been disabled since June 2008; that code only clutters the implementation we actually use. --- indra/llcommon/llrand.cpp | 46 +++------------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 4e4345f37a..33afc50cf7 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,48 +58,12 @@ * to restore uniform distribution. */ +static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); + +// no default implementation, only specific F64 and F32 specializations template inline REAL ll_internal_random(); -// *NOTE: The system rand implementation is probably not correct. -#define LL_USE_SYSTEM_RAND 0 - -/***************************************************************************** -* The LL_USE_SYSTEM_RAND implementation has been disabled since June 2008. -*****************************************************************************/ -#if LL_USE_SYSTEM_RAND -#include - -class LLSeedRand -{ -public: - LLSeedRand() - { -#if LL_WINDOWS - srand(LLUUID::getRandomSeed()); -#else - srand48(LLUUID::getRandomSeed()); -#endif - } -}; -static LLSeedRand sRandomSeeder; - -template <> -inline F64 ll_internal_random() -{ -#if LL_WINDOWS - return (F64)rand() / (F64)(RAND_MAX+1); -#else - return drand48(); -#endif -} - -/***************************************************************************** -* This is the implementation we've been using. -*****************************************************************************/ -#else // LL_USE_SYSTEM_RAND -static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); - template <> inline F64 ll_internal_random() { @@ -111,11 +75,7 @@ inline F64 ll_internal_random() if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } -#endif // LL_USE_SYSTEM_RAND -/***************************************************************************** -* Functions common to both implementations -*****************************************************************************/ template <> inline F32 ll_internal_random() { -- cgit v1.2.3 From 25efba151f98308a0e2d9af52a76173be3f8aa04 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 29 Aug 2023 11:12:32 -0400 Subject: SL-18837: On Windows, LLLeap partial final line test failed. Add DEBUG log output to try to diagnose. --- indra/llcommon/llleap.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index c87c0758fe..060e96557d 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -376,6 +376,17 @@ public: // Read all remaining bytes and log. LL_INFOS("LLLeap") << mDesc << ": " << rest << LL_ENDL; } + /*--------------------------- diagnostic ---------------------------*/ + else if (data["eof"].asBoolean()) + { + LL_DEBUGS("LLLeap") << mDesc << " ended, no partial line" << LL_ENDL; + } + else + { + LL_DEBUGS("LLLeap") << mDesc << " (still running, " << childerr.size() + << " bytes pending)" << LL_ENDL; + } + /*------------------------- end diagnostic -------------------------*/ return false; } -- cgit v1.2.3 From 6a219d147d0761fce1fd75dc75f98b4e5283feb7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Aug 2023 15:32:35 -0400 Subject: SL-18837: Enlarge default coroutine stack size. A test executable on a GitHub Windows runner failed with C00000FD, which reports stack overflow. (cherry picked from commit aab7b4ba3812e5876b1205285bcfd8cff96bcac9) --- indra/llcommon/llcoros.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 70d8dfc8b9..cfaf3415e7 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -123,11 +123,7 @@ LLCoros::LLCoros(): // Previously we used // boost::context::guarded_stack_allocator::default_stacksize(); // empirically this is insufficient. -#if ADDRESS_SIZE == 64 - mStackSize(512*1024), -#else - mStackSize(256*1024), -#endif + mStackSize(768*1024), // mCurrent does NOT own the current CoroData instance -- it simply // points to it. So initialize it with a no-op deleter. mCurrent{ [](CoroData*){} } -- cgit v1.2.3 From 95aa00f7427b7d19ab502862b3018012c1bf1904 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 7 Sep 2023 13:40:46 -0400 Subject: SL-18837: Fix minor merge glitch. --- indra/llcommon/tests/llsdserialize_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 2de7df6f36..76e9ecc293 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -2081,7 +2081,7 @@ namespace tut " for item in DATA:\n" " serialized = llsd." << pyformatter << "(item)\n" " f.write(lenformat.pack(len(serialized)))\n" - " f.write(serialized)\n"); + " f.write(serialized)\n";}); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From a45c9f68c3e2600f48b25cc5cc74ef47cd83005b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 08:58:32 -0400 Subject: SL-18837: Add debugging output to llsdserialize_test.cpp. --- indra/llcommon/tests/llsdserialize_test.cpp | 90 ++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 76e9ecc293..ca63e74c6c 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -55,7 +55,9 @@ typedef U32 uint32_t; #include "../test/lltut.h" #include "../test/namedtempfile.h" #include "stringize.h" +#include #include +#include typedef std::function FormatterFunction; typedef std::function ParserFunction; @@ -65,6 +67,81 @@ std::vector string_to_vector(const std::string& str) return std::vector(str.begin(), str.end()); } +// Format a given byte string as 2-digit hex values, no separators +// Usage: std::cout << hexdump(somestring) << ... +class hexdump +{ +public: + hexdump(const char* data, size_t len): + hexdump(reinterpret_cast(data), len) + {} + + hexdump(const U8* data, size_t len): + mData(data, data + len) + {} + + hexdump(const hexdump&) = delete; + hexdump& operator=(const hexdump&) = delete; + + friend std::ostream& operator<<(std::ostream& out, const hexdump& self) + { + auto oldfmt{ out.flags() }; + auto oldfill{ out.fill() }; + out.setf(std::ios_base::hex, std::ios_base::basefield); + out.fill('0'); + for (auto c : self.mData) + { + out << std::setw(2) << unsigned(c); + } + out.setf(oldfmt, std::ios_base::basefield); + out.fill(oldfill); + return out; + } + +private: + std::vector mData; +}; + +// Format a given byte string as a mix of printable characters and, for each +// non-printable character, "\xnn" +// Usage: std::cout << hexmix(somestring) << ... +class hexmix +{ +public: + hexmix(const char* data, size_t len): + mData(data, len) + {} + + hexmix(const hexmix&) = delete; + hexmix& operator=(const hexmix&) = delete; + + friend std::ostream& operator<<(std::ostream& out, const hexmix& self) + { + auto oldfmt{ out.flags() }; + auto oldfill{ out.fill() }; + out.setf(std::ios_base::hex, std::ios_base::basefield); + out.fill('0'); + for (auto c : self.mData) + { + // std::isprint() must be passed an unsigned char! + if (std::isprint(static_cast(c))) + { + out << c; + } + else + { + out << "\\x" << std::setw(2) << unsigned(c); + } + } + out.setf(oldfmt, std::ios_base::basefield); + out.fill(oldfill); + return out; + } + +private: + std::string mData; +}; + namespace tut { struct sd_xml_data @@ -1909,7 +1986,14 @@ namespace tut auto buffstr{ buffer.str() }; int bufflen{ static_cast(buffstr.length()) }; out.write(reinterpret_cast(&bufflen), sizeof(bufflen)); + LL_DEBUGS("topy") << "Wrote length: " + << hexdump(reinterpret_cast(&bufflen), + sizeof(bufflen)) + << LL_ENDL; out.write(buffstr.c_str(), buffstr.length()); + LL_DEBUGS("topy") << "Wrote data: " + << hexmix(buffstr.c_str(), buffstr.length()) + << LL_ENDL; } } @@ -1938,8 +2022,8 @@ namespace tut " else:\n" " raise AssertionError('Too many data items')\n"; - // Create an llsdXXXXXX file containing 'data' serialized to - // notation. + // Create an llsdXXXXXX file containing 'data' serialized per + // FormatterFunction. NamedTempFile file("llsd", // NamedTempFile's function constructor // takes a callable. To this callable it passes the @@ -1958,11 +2042,13 @@ namespace tut "lenformat = struct.Struct('i')\n" "def parse_each(inf):\n" " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" + " print('Read length:', ''.join(('%02x' % b) for b in rawlen))\n" " len = lenformat.unpack(rawlen)[0]\n" // Since llsd.parse() has no max_bytes argument, instead of // passing the input stream directly to parse(), read the item // into a distinct bytes object and parse that. " data = inf.read(len)\n" + " print('Read data: ', repr(data))\n" " try:\n" " frombytes = llsd.parse(data)\n" " except llsd.LLSDParseError as err:\n" -- cgit v1.2.3 From c7546ea65e55143ff3d2d82d8c289bbac7fffe0f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 14:14:09 -0400 Subject: SL-18837: Make llsdserialize_test debug output conditional. Move hexdump() and hexmix() stream formatters to new hexdump.h for potential use by other tests. In toPythonUsing() helper function, add a temp file to receive Python script debug output, and direct debug output to that file. On test failure, dump the contents of that file to the log. Give NamedTempFile::peep() an optional target std::ostream; refactor implementation as peep_via() that accepts a callable to process each text line. Add operator<<() to stream the contents of a NamedTempFile object to ostream -- but don't use that with LL_DEBUGS(), as it flattens the file contents into a single log line. Instead add peep_log(), which streams each individual text line to LL_DEBUGS(). --- indra/llcommon/tests/llsdserialize_test.cpp | 202 +++++++++++----------------- 1 file changed, 75 insertions(+), 127 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index ca63e74c6c..ac40125f75 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -52,12 +52,12 @@ typedef U32 uint32_t; #include "llformat.h" #include "llmemorystream.h" +#include "../test/hexdump.h" #include "../test/lltut.h" #include "../test/namedtempfile.h" #include "stringize.h" -#include +#include "StringVec.h" #include -#include typedef std::function FormatterFunction; typedef std::function ParserFunction; @@ -67,81 +67,6 @@ std::vector string_to_vector(const std::string& str) return std::vector(str.begin(), str.end()); } -// Format a given byte string as 2-digit hex values, no separators -// Usage: std::cout << hexdump(somestring) << ... -class hexdump -{ -public: - hexdump(const char* data, size_t len): - hexdump(reinterpret_cast(data), len) - {} - - hexdump(const U8* data, size_t len): - mData(data, data + len) - {} - - hexdump(const hexdump&) = delete; - hexdump& operator=(const hexdump&) = delete; - - friend std::ostream& operator<<(std::ostream& out, const hexdump& self) - { - auto oldfmt{ out.flags() }; - auto oldfill{ out.fill() }; - out.setf(std::ios_base::hex, std::ios_base::basefield); - out.fill('0'); - for (auto c : self.mData) - { - out << std::setw(2) << unsigned(c); - } - out.setf(oldfmt, std::ios_base::basefield); - out.fill(oldfill); - return out; - } - -private: - std::vector mData; -}; - -// Format a given byte string as a mix of printable characters and, for each -// non-printable character, "\xnn" -// Usage: std::cout << hexmix(somestring) << ... -class hexmix -{ -public: - hexmix(const char* data, size_t len): - mData(data, len) - {} - - hexmix(const hexmix&) = delete; - hexmix& operator=(const hexmix&) = delete; - - friend std::ostream& operator<<(std::ostream& out, const hexmix& self) - { - auto oldfmt{ out.flags() }; - auto oldfill{ out.fill() }; - out.setf(std::ios_base::hex, std::ios_base::basefield); - out.fill('0'); - for (auto c : self.mData) - { - // std::isprint() must be passed an unsigned char! - if (std::isprint(static_cast(c))) - { - out << c; - } - else - { - out << "\\x" << std::setw(2) << unsigned(c); - } - } - out.setf(oldfmt, std::ios_base::basefield); - out.fill(oldfill); - return out; - } - -private: - std::string mData; -}; - namespace tut { struct sd_xml_data @@ -1868,16 +1793,12 @@ namespace tut // helper for TestPythonCompatible static std::string import_llsd("import os.path\n" "import sys\n" - "try:\n" - // new freestanding llsd package - " import llsd\n" - "except ImportError:\n" - // older llbase.llsd module - " from llbase import llsd\n"); + "import llsd\n"); // helper for TestPythonCompatible - template - void python(const std::string& desc, const CONTENT& script, int expect=0) + template + void python_expect(const std::string& desc, const CONTENT& script, int expect=0, + ARGS&&... args) { auto PYTHON(LLStringUtil::getenv("PYTHON")); ensure("Set $PYTHON to the Python interpreter", !PYTHON.empty()); @@ -1888,7 +1809,8 @@ namespace tut std::string q("\""); std::string qPYTHON(q + PYTHON + q); std::string qscript(q + scriptfile.getName() + q); - int rc = _spawnl(_P_WAIT, PYTHON.c_str(), qPYTHON.c_str(), qscript.c_str(), NULL); + int rc = _spawnl(_P_WAIT, PYTHON.c_str(), qPYTHON.c_str(), qscript.c_str(), + std::forward(args)..., NULL); if (rc == -1) { char buffer[256]; @@ -1904,6 +1826,10 @@ namespace tut LLProcess::Params params; params.executable = PYTHON; params.args.add(scriptfile.getName()); + for (const std::string& arg : StringVec{ std::forward(args)... }) + { + params.args.add(arg); + } LLProcessPtr py(LLProcess::create(params)); ensure(STRINGIZE("Couldn't launch " << desc << " script"), bool(py)); // Implementing timeout would mean messing with alarm() and @@ -1938,6 +1864,14 @@ namespace tut #endif } + // helper for TestPythonCompatible + template + void python(const std::string& desc, const CONTENT& script, ARGS&&... args) + { + // plain python() expects rc 0 + python_expect(desc, script, 0, std::forward(args)...); + } + struct TestPythonCompatible { TestPythonCompatible() {} @@ -1952,10 +1886,10 @@ namespace tut void TestPythonCompatibleObject::test<1>() { set_test_name("verify python()"); - python("hello", - "import sys\n" - "sys.exit(17)\n", - 17); // expect nonzero rc + python_expect("hello", + "import sys\n" + "sys.exit(17)\n", + 17); // expect nonzero rc } template<> template<> @@ -1986,14 +1920,14 @@ namespace tut auto buffstr{ buffer.str() }; int bufflen{ static_cast(buffstr.length()) }; out.write(reinterpret_cast(&bufflen), sizeof(bufflen)); - LL_DEBUGS("topy") << "Wrote length: " - << hexdump(reinterpret_cast(&bufflen), - sizeof(bufflen)) - << LL_ENDL; + LL_DEBUGS() << "Wrote length: " + << hexdump(reinterpret_cast(&bufflen), + sizeof(bufflen)) + << LL_ENDL; out.write(buffstr.c_str(), buffstr.length()); - LL_DEBUGS("topy") << "Wrote data: " - << hexmix(buffstr.c_str(), buffstr.length()) - << LL_ENDL; + LL_DEBUGS() << "Wrote data: " + << hexmix(buffstr.c_str(), buffstr.length()) + << LL_ENDL; } } @@ -2033,36 +1967,50 @@ namespace tut (std::ostream& out) { writeLLSDArray(serialize, out, cdata); }); - python("read C++ " + desc, - [&](std::ostream& out){ out << - import_llsd << - "from functools import partial\n" - "import io\n" - "import struct\n" - "lenformat = struct.Struct('i')\n" - "def parse_each(inf):\n" - " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" - " print('Read length:', ''.join(('%02x' % b) for b in rawlen))\n" - " len = lenformat.unpack(rawlen)[0]\n" - // Since llsd.parse() has no max_bytes argument, instead of - // passing the input stream directly to parse(), read the item - // into a distinct bytes object and parse that. - " data = inf.read(len)\n" - " print('Read data: ', repr(data))\n" - " try:\n" - " frombytes = llsd.parse(data)\n" - " except llsd.LLSDParseError as err:\n" - " print(f'*** {err}')\n" - " print(f'Bad content:\\n{data!r}')\n" - " raise\n" - // Also try parsing from a distinct stream. - " stream = io.BytesIO(data)\n" - " fromstream = llsd.parse(stream)\n" - " assert frombytes == fromstream\n" - " yield frombytes\n" - << pydata << - // Don't forget raw-string syntax for Windows pathnames. - "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n";}); + // 'debug' starts empty because it's intended as an output file + NamedTempFile debug("debug", ""); + + try + { + python("read C++ " + desc, + [&](std::ostream& out){ out << + import_llsd << + "from functools import partial\n" + "import io\n" + "import struct\n" + "lenformat = struct.Struct('i')\n" + "def parse_each(inf):\n" + " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" + " print('Read length:', ''.join(('%02x' % b) for b in rawlen),\n" + " file=debug)\n" + " len = lenformat.unpack(rawlen)[0]\n" + // Since llsd.parse() has no max_bytes argument, instead of + // passing the input stream directly to parse(), read the item + // into a distinct bytes object and parse that. + " data = inf.read(len)\n" + " print('Read data: ', repr(data), file=debug)\n" + " try:\n" + " frombytes = llsd.parse(data)\n" + " except llsd.LLSDParseError as err:\n" + " print(f'*** {err}')\n" + " print(f'Bad content:\\n{data!r}')\n" + " raise\n" + // Also try parsing from a distinct stream. + " stream = io.BytesIO(data)\n" + " fromstream = llsd.parse(stream)\n" + " assert frombytes == fromstream\n" + " yield frombytes\n" + << pydata << + // Don't forget raw-string syntax for Windows pathnames. + "debug = open(r'" << debug.getName() << "', 'w')\n" + "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n";}); + } + catch (const failure&) + { + LL_DEBUGS() << "Script debug output:" << LL_ENDL; + debug.peep_log(); + throw; + } } template<> template<> -- cgit v1.2.3 From 7504b1c319373c950e8b8c2c7a8b2f0d9abf1d8b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 5 Oct 2023 10:17:09 -0400 Subject: SL-18837: When llrand_test.cpp fails, display the failing value. It's frustrating and unactionable to have a failing test report merely that the random value was greater than the specified high end. Okay, so what was the value? If it's supposed to be less than the high end, did it happen to be equal? Or was it garbage? We can't reproduce the failure by rerunning! The new ensure_in_exc_range(), ensure_in_inc_range() mechanism is somewhat complex because exactly one test allows equality with the high end of the expected range, where the rest mandate that the function return less than the high end. If that's a bug in the test -- if every llrand function is supposed to return less than the high end -- then we could simplify the test logic. --- indra/llcommon/tests/llrand_test.cpp | 60 +++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 28 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llrand_test.cpp b/indra/llcommon/tests/llrand_test.cpp index 383e6f9e0a..c8e2d19372 100644 --- a/indra/llcommon/tests/llrand_test.cpp +++ b/indra/llcommon/tests/llrand_test.cpp @@ -29,7 +29,30 @@ #include "../test/lltut.h" #include "../llrand.h" +#include "stringize.h" +template +void ensure_in_range_using(const std::string_view& name, + NUMBER value, NUMBER low, NUMBER high, + const std::string_view& compdesc, HIGHCOMP&& highcomp) +{ + auto failmsg{ stringize(name, " >= ", low, " (", value, ')') }; + tut::ensure(failmsg, (value >= low)); + failmsg = stringize(name, ' ', compdesc, ' ', high, " (", value, ')'); + tut::ensure(failmsg, std::forward(highcomp)(value, high)); +} + +template +void ensure_in_exc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) +{ + ensure_in_range_using(name, value, low, high, "<", std::less()); +} + +template +void ensure_in_inc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) +{ + ensure_in_range_using(name, value, low, high, "<=", std::less_equal()); +} namespace tut { @@ -44,84 +67,65 @@ namespace tut template<> template<> void random_object_t::test<1>() { - F32 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_frand(); - ensure("frand >= 0", (number >= 0.0f)); - ensure("frand < 1", (number < 1.0f)); + ensure_in_exc_range("frand", ll_frand(), 0.0f, 1.0f); } } template<> template<> void random_object_t::test<2>() { - F64 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_drand(); - ensure("drand >= 0", (number >= 0.0)); - ensure("drand < 1", (number < 1.0)); + ensure_in_exc_range("drand", ll_drand(), 0.0, 1.0); } } template<> template<> void random_object_t::test<3>() { - F32 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_frand(2.0f) - 1.0f; - ensure("frand >= 0", (number >= -1.0f)); - ensure("frand < 1", (number <= 1.0f)); + ensure_in_inc_range("frand(2.0f)", ll_frand(2.0f) - 1.0f, -1.0f, 1.0f); } } template<> template<> void random_object_t::test<4>() { - F32 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_frand(-7.0); - ensure("drand <= 0", (number <= 0.0)); - ensure("drand > -7", (number > -7.0)); + // Negate the result so we don't have to allow a templated low-end + // comparison as well. + ensure_in_exc_range("-frand(-7.0)", -ll_frand(-7.0), 0.0f, 7.0f); } } template<> template<> void random_object_t::test<5>() { - F64 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_drand(-2.0); - ensure("drand <= 0", (number <= 0.0)); - ensure("drand > -2", (number > -2.0)); + ensure_in_exc_range("-drand(-2.0)", -ll_drand(-2.0), 0.0, 2.0); } } template<> template<> void random_object_t::test<6>() { - S32 number = 0; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_rand(100); - ensure("rand >= 0", (number >= 0)); - ensure("rand < 100", (number < 100)); + ensure_in_exc_range("rand(100)", ll_rand(100), 0, 100); } } template<> template<> void random_object_t::test<7>() { - S32 number = 0; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_rand(-127); - ensure("rand <= 0", (number <= 0)); - ensure("rand > -127", (number > -127)); + ensure_in_exc_range("-rand(-127)", -ll_rand(-127), 0, 127); } } } -- cgit v1.2.3 From f5a34fd074bda091732a8ae0a4cf6f4e0358a140 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 12 Oct 2023 16:55:04 -0400 Subject: SL-18837: Unify all llrand_test.cpp in-range tests. The header file documents that no llrand function should ever return a value equal to the passed extent, so the one test in llrand_test.cpp that checked less than or equal to the high end of the range was anomalous. But changing that to an exclusive range means that we no longer need separate exclusive range and inclusive range functions. Replace ensure_in_range_using(), ensure_in_exc_range() and ensure_in_inc_range() with a grand unified (simplified) ensure_in_range() function. --- indra/llcommon/tests/llrand_test.cpp | 43 +++++++++++++++--------------------- 1 file changed, 18 insertions(+), 25 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llrand_test.cpp b/indra/llcommon/tests/llrand_test.cpp index c8e2d19372..ac5a33d0ba 100644 --- a/indra/llcommon/tests/llrand_test.cpp +++ b/indra/llcommon/tests/llrand_test.cpp @@ -31,27 +31,20 @@ #include "../llrand.h" #include "stringize.h" -template -void ensure_in_range_using(const std::string_view& name, - NUMBER value, NUMBER low, NUMBER high, - const std::string_view& compdesc, HIGHCOMP&& highcomp) +// In llrand.h, every function is documented to return less than the high end +// -- specifically, because you can pass a negative extent, they're documented +// never to return a value equal to the extent. +// So that we don't need two different versions of ensure_in_range(), when +// testing extent < 0, negate the return value and the extent before passing +// into ensure_in_range(). +template +void ensure_in_range(const std::string_view& name, + NUMBER value, NUMBER low, NUMBER high) { auto failmsg{ stringize(name, " >= ", low, " (", value, ')') }; tut::ensure(failmsg, (value >= low)); - failmsg = stringize(name, ' ', compdesc, ' ', high, " (", value, ')'); - tut::ensure(failmsg, std::forward(highcomp)(value, high)); -} - -template -void ensure_in_exc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) -{ - ensure_in_range_using(name, value, low, high, "<", std::less()); -} - -template -void ensure_in_inc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) -{ - ensure_in_range_using(name, value, low, high, "<=", std::less_equal()); + failmsg = stringize(name, " < ", high, " (", value, ')'); + tut::ensure(failmsg, (value < high)); } namespace tut @@ -69,7 +62,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("frand", ll_frand(), 0.0f, 1.0f); + ensure_in_range("frand", ll_frand(), 0.0f, 1.0f); } } @@ -78,7 +71,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("drand", ll_drand(), 0.0, 1.0); + ensure_in_range("drand", ll_drand(), 0.0, 1.0); } } @@ -87,7 +80,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_inc_range("frand(2.0f)", ll_frand(2.0f) - 1.0f, -1.0f, 1.0f); + ensure_in_range("frand(2.0f)", ll_frand(2.0f) - 1.0f, -1.0f, 1.0f); } } @@ -98,7 +91,7 @@ namespace tut { // Negate the result so we don't have to allow a templated low-end // comparison as well. - ensure_in_exc_range("-frand(-7.0)", -ll_frand(-7.0), 0.0f, 7.0f); + ensure_in_range("-frand(-7.0)", -ll_frand(-7.0), 0.0f, 7.0f); } } @@ -107,7 +100,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("-drand(-2.0)", -ll_drand(-2.0), 0.0, 2.0); + ensure_in_range("-drand(-2.0)", -ll_drand(-2.0), 0.0, 2.0); } } @@ -116,7 +109,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("rand(100)", ll_rand(100), 0, 100); + ensure_in_range("rand(100)", ll_rand(100), 0, 100); } } @@ -125,7 +118,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("-rand(-127)", -ll_rand(-127), 0, 127); + ensure_in_range("-rand(-127)", -ll_rand(-127), 0, 127); } } } -- cgit v1.2.3 From 651353560bfe23b6423ecf7690d86645a71c0cbc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Oct 2023 14:56:10 -0400 Subject: SL-20476: Don't let the compiler know we intend to crash. clang has gotten smart enough to recognize an inline attempt to store to address zero. Fool it by storing to an address passed as a parameter, and pass nullptr from a different source file. --- indra/llcommon/llerror.cpp | 17 +++++++++++++++-- indra/llcommon/llerror.h | 10 +++++----- 2 files changed, 20 insertions(+), 7 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 02cb186275..05e719b494 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -1617,5 +1617,18 @@ bool debugLoggingEnabled(const std::string& tag) return res; } - - +void crashdriver(void (*callback)(int*)) +{ + // The LLERROR_CRASH macro used to have inline code of the form: + //int* make_me_crash = NULL; + //*make_me_crash = 0; + + // But compilers are getting smart enough to recognize that, so we must + // assign to an address supplied by a separate source file. We could do + // the assignment here in crashdriver() -- but then BugSplat would group + // all LL_ERRS() crashes as the fault of this one function, instead of + // identifying the specific LL_ERRS() source line. So instead, do the + // assignment in a lambda in the caller's source. We just provide the + // nullptr target. + callback(nullptr); +} diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 020f05e8f5..624a5fb37a 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -383,11 +383,9 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' // Use this only in LL_ERRS or in a place that LL_ERRS may not be used -#define LLERROR_CRASH \ -{ \ - int* make_me_crash = NULL;\ - *make_me_crash = 0; \ - exit(*make_me_crash); \ +#define LLERROR_CRASH \ +{ \ + crashdriver([](int* ptr){ *ptr = 0; exit(*ptr); }); \ } #define LL_ENDL \ @@ -466,5 +464,7 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; // Check at run-time whether logging is enabled, without generating output bool debugLoggingEnabled(const std::string& tag); +// used by LLERROR_CRASH +void crashdriver(void (*)(int*)); #endif // LL_LLERROR_H -- cgit v1.2.3