00:00:52 I pushed it to branch so other people can review it, but I do think that code's ready for master. 00:08:00 not sure if I'm telling you something you know how to do, but you can add the test to travis in your branch and it will work before you merge 00:16:19 I suspected so. Although I can't actually find much useful info on how to modify crawl's travis file. I think I have a patch that will work based on what's already present, but I'm not certain. 00:24:56 https://pastebin.com/0LpYHWvP I'll wait to push until the current travis builds finish. That way, in case I somehow broke the current builds I'll know it wasn't my .travis modifications. 00:36:20 The build passed. (catch2-integration - a901c2c #12450 : Seve Monahan): https://travis-ci.org/crawl/crawl/builds/628583323 01:01:34 03reaverb02 07[catch2-integration] * 0.25-a0-312-g5769b18: Add automatic catch2 tests to travis-ci builds 10(44 minutes ago, 2 files, 8+ 0-) 13https://github.com/crawl/crawl/commit/5769b184a261 01:37:31 Unstable branch on crawl.develz.org updated to: 0.25-a0-310-g60f0ac2 (34) 01:54:32 Where are the inventory menu colors defined? (Like that acquirement is Cyan, etc.) I found the highlight colour in item-name.cc but that doesn't affect it most the time. 02:01:55 menu_colours.txt found it. O_O; why is that set out here while the highlight colour is in with the rest of itemname? 02:09:22 Windows builds of master branch on crawl.develz.org updated to: 0.25-a0-310-g60f0ac2 02:58:35 Monster database of master branch on crawl.develz.org updated to: 0.24-a0-443-g80245de 03:02:58 nice, getting a new feature into travis with a single commit 03:11:46 Unstable branch on crawl.beRotato.org updated to: 0.25-a0-310-g60f0ac2 (34) 03:32:24 Fork (bcadrencrawl) on crawl.kelbi.org updated to: 0.22.1-1861-g1725a49d29 04:40:10 !tell reaverb ./AppHdr.h:167:6: error: Missing platform #define or unsupported compiler. from catch2-tests/*.cc on osx when running make catch2-tests 04:40:10 alexjurkiewicz: OK, I'll let reaverb know. 05:33:30 -!- amalloy is now known as amalloy_ 08:31:39 Trog does not present artifact weapons. 13https://crawl.develz.org/mantis/view.php?id=12159 by sdynet 09:23:11 !tell alexjurkiewicz Thanks for the feedback! Could you check if "make monster" or "make test-monster" have the same issue on crawl's *master* branch? That might be an issue with includes using paths relative from /source rather than the file location, in which case master monster would share those issues. 09:23:11 reaverb: OK, I'll let alexjurkiewicz know. 09:26:30 aidanh: you may be interested in this: https://github.com/crawl/crawl/issues/1224 09:26:36 was also reported in discord by kaninmat, mentioned he has 'message_colour ^= lightgreen:Found' in his rc 09:26:50 maybe related to this? https://github.com/crawl/crawl/commit/dbf363a7764abcd79cc012143a4f204229082a87 09:46:17 -!- bairyn is now known as ByronJohnson 11:15:51 !tell alexjurkiewicz Just to be clear, I don't consider failing OS X compilation to be a reason to hold catch2_integration back from master. It's definitely not ideal and worth fixing as soon as possible, but I think catch2 tests on Windows and Linux but not Mac > no catch2 tests for anybody. 11:15:52 reaverb: OK, I'll let alexjurkiewicz know. 11:23:27 Stable (0.23) branch on underhound.eu updated to: 0.23.1-91-gf373564dc4 12:17:12 03reaverb02 07[catch2-integration] * 0.25-a0-313-g9aa3435: Test whether travis-ci can detect failing catch2 tests 10(11 hours ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/9aa3435fcc3c 12:57:13 -!- amalloy_ is now known as amalloy 12:58:20 The build was broken. (catch2-integration - 9aa3435 #12452 : Seve Monahan): https://travis-ci.org/crawl/crawl/builds/628807003 13:07:44 Excellent, travis-ci will report failing catch2 tests. 13:08:33 03reaverb02 07[catch2-integration] * 0.25-a0-314-ge4868ce: Revert "Test whether travis-ci can detect failing catch2 tests" 10(51 minutes ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/e4868ce5c1a4 13:49:52 The build was fixed. (catch2-integration - e4868ce #12453 : Seve Monahan): https://travis-ci.org/crawl/crawl/builds/628825566 14:00:59 reaverb: sure 14:00:59 alexjurkiewicz: You have 2 messages. Use !messages to read them. 14:03:25 reaverb: also, are you planning to add test coverage? I got coverage working on a branch if youd like a PR 14:10:02 test coverage sounds great, please make a pull request for it. 14:13:08 make monster alsow roks fwiw 14:17:17 weird that it's failing on TARGET_OS_MACOSX. It's defined by platform.h which is included from AppHdr.h 14:17:52 Hmm, if make monster works my next guess is something in catch.hpp somehow has a name conflicts with AppHdr.h. Which isn't something I feel I can debug without having a mac to work on. 14:22:14 Just to be clear, alexjurkiewicz, you did mean that "make monster" can compile on mac os x? I think that's what you meant, but "alsow roks" is a little ambiguous. 14:22:55 i did mean that 14:23:12 it looks like catch.hpp includes /usr/include/TargetConditionals.h which defines overlapping define var 14:23:53 Ah, that should be fixable by #undefing it at the end of catch.hpp. I already had to do that for one name conflict. 14:24:26 "it" being "whichever name(s) are causing the conflict" 14:24:41 ok i'll do some tests 14:25:22 edit that file directly? I thought it was a copy of the file from catch2. Maybe there should be a post header for crawl-specific tweaks? 14:27:16 I edited the file directly, but now that you mention it putting the crawl-specific tweaks somewhere else is a much better idea. 14:27:39 Perhaps a "crawl-catch.hpp" file which #includes catch.hpp then has the crawl specific tweaks. 14:31:47 adding #define TARGET_OS_MACOSX works, so your idea is right. I'll try to isolate it properly 14:36:18 Maybe see if CATCH_CONFIG_PREFIX_ALL works? https://github.com/catchorg/Catch2/blob/master/docs/configuration.md There's probably some really fancy solution that completely isolates the catch.hpp stuff, but I think it's fine to use a good-enough solution and make a prefect solution if and when it's needed. 14:38:47 just adding a little tweak to platform.h which should do it 14:40:34 yep. here's a fix for you, in platform.h add the second test #if defined (__APPLE__) || defined (TARGET_OS_OSX) 14:42:23 Can you make a patch or do a pull request? If I put the test in the wrong place I would have no way to know. Also, you might have already done this, but it's good to make sure normal crawl compilation still works after your change. 14:42:52 Thanks for taking the time to debug this! I really appreciate it. 14:57:51 alexjurkiewicz: ^ Sorry if you already saw the above, I know some people like to be pinged whenever they are addressed. 15:32:04 sure thing 15:32:42 yeah please ping me if it's been hours since a chat. But I'm easy on the notification style otherwise 16:09:20 -!- werekitten is now known as misha 17:29:00 -!- amalloy is now known as amalloy_ 18:08:10 heh. Somehow catch2 is causing platform.h to detect the OS as TARGET_OS_NDSFIRMWARE 18:35:53 reaverb: I can't set you as reviewer, but here: https://github.com/crawl/crawl/pull/1239 18:38:42 I am extremely surprised Crawl has Nintendo DS support. 18:41:01 New branch created: pull/1239 (5 commits) 13https://github.com/crawl/crawl/pull/1239 18:41:01 03reaverb02 07https://github.com/crawl/crawl/pull/1239 * 0.25-a0-311-ga901c2c: Integrate catch2 testing framework into crawl. 10(19 hours ago, 11 files, 17804+ 65-) 13https://github.com/crawl/crawl/commit/a901c2c2c741 18:41:01 03reaverb02 07https://github.com/crawl/crawl/pull/1239 * 0.25-a0-312-g5769b18: Add automatic catch2 tests to travis-ci builds 10(18 hours ago, 2 files, 8+ 0-) 13https://github.com/crawl/crawl/commit/5769b184a261 18:41:01 03reaverb02 07https://github.com/crawl/crawl/pull/1239 * 0.25-a0-313-g9aa3435: Test whether travis-ci can detect failing catch2 tests 10(17 hours ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/9aa3435fcc3c 18:41:01 03reaverb02 07https://github.com/crawl/crawl/pull/1239 * 0.25-a0-314-ge4868ce: Revert "Test whether travis-ci can detect failing catch2 tests" 10(6 hours ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/e4868ce5c1a4 18:41:01 03Alex Jurkiewicz02 07https://github.com/crawl/crawl/pull/1239 * 0.25-a0-315-g56d087c: Fix macOS OS detection with catch2 10(12 hours ago, 1 file, 7+ 7-) 13https://github.com/crawl/crawl/commit/56d087c9bcb1 18:41:55 Probably better to #undef catch2's TARGET_CPU_ARM instead of changing to order of the checks, don't want to cause a confusing problem where somebody tries to compile catch2-tests in a weird environment and ends up trigger the Nintendo DS code. 18:43:06 I was hoping to avoid that because I think a bunch of random TARGET_CPU_* defines have been set 18:43:20 so I probably have to check every define in platform.h D: 18:45:49 actually, that isn't so hard... 19:27:58 alexjurkiewicz: I think that your pull request is worth merging into master. Do you want to do the TARGET_CPU_* #undefs before merging into master or after? Same question for implementing test coverage (which isn't on your pull request, although I recall you saying test coverage was working on a branch?) 19:31:52 yeah. I stopped working on the TARGET_CPU fix while I did some more coverage work. Just getting the HTML coverage report pretty now 19:32:21 I have to base this PR off the fix to TARGET_CPU. So I'll finish coverage, then improve the CPU fix, then bundle it all into one PR 19:32:29 I'll update the existing PR when that's ready 19:38:01 I'm going to merge your PR as-is in a couple of hours because I'll have inconsistent computer access the next few days. If the test converge / macro undefs aren't done by then, make a new PR and I'll look at it after the holidays. 19:45:01 ok 19:53:32 pushed the coverage PR, now looking at the CPU fix 19:53:41 cpp coverage is tricky 19:56:00 New branch created: pull/1240 (6 commits) 13https://github.com/crawl/crawl/pull/1240 19:56:00 03reaverb02 07https://github.com/crawl/crawl/pull/1240 * 0.25-a0-311-ga901c2c: Integrate catch2 testing framework into crawl. 10(20 hours ago, 11 files, 17804+ 65-) 13https://github.com/crawl/crawl/commit/a901c2c2c741 19:56:00 03reaverb02 07https://github.com/crawl/crawl/pull/1240 * 0.25-a0-312-g5769b18: Add automatic catch2 tests to travis-ci builds 10(20 hours ago, 2 files, 8+ 0-) 13https://github.com/crawl/crawl/commit/5769b184a261 19:56:00 03reaverb02 07https://github.com/crawl/crawl/pull/1240 * 0.25-a0-313-g9aa3435: Test whether travis-ci can detect failing catch2 tests 10(19 hours ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/9aa3435fcc3c 19:56:00 03reaverb02 07https://github.com/crawl/crawl/pull/1240 * 0.25-a0-314-ge4868ce: Revert "Test whether travis-ci can detect failing catch2 tests" 10(8 hours ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/e4868ce5c1a4 19:56:00 03Alex Jurkiewicz02 07https://github.com/crawl/crawl/pull/1240 * 0.25-a0-315-g56d087c: Fix macOS OS detection with catch2 10(13 hours ago, 1 file, 7+ 7-) 13https://github.com/crawl/crawl/commit/56d087c9bcb1 19:56:00 03Alex Jurkiewicz02 07https://github.com/crawl/crawl/pull/1240 * 0.25-a0-316-g6f5d2c3: Add code coverage tracking 10(13 hours ago, 3 files, 17+ 1-) 13https://github.com/crawl/crawl/commit/6f5d2c34f742 20:03:48 yeah, somehow catch2.hpp causes TARGET_CPU_ALPHA, _ARM, _MIPS, _SPARC, _X86, _X86_64 to all be defined. I have no idea how, so I'm going to suggest sticking with the original fix. Especially since nothing in crawl depends on those defines 20:04:44 actually. it will break a few random things. Like BACKTRACE_SUPPORTED will be disabled during catch2 tests. So it should be fixed eventually. But I think the current PR is correct 20:05:19 both PRs good to merge IMO 22:43:06 -!- amalloy_ is now known as amalloy