labath added a comment. In D100191#2704492 <https://reviews.llvm.org/D100191#2704492>, @mgorny wrote:
> In D100191#2704403 <https://reviews.llvm.org/D100191#2704403>, @labath wrote: > >> Let's identify the set of patches needed to make this testable via the >> lldb-server suite (this one, D100153 <https://reviews.llvm.org/D100153>, >> D100208 <https://reviews.llvm.org/D100208> (or equivalent for some other >> os), and what else?) and test that? > > In its current form, D100208 <https://reviews.llvm.org/D100208> relies at > least on D100196 <https://reviews.llvm.org/D100196> as well. I suppose we > might get away without other patches for now. My logic is that as long as > client doesn't indicate fork support, the regular LLDB behavior won't change. > We can mock-enable `fork-events` in the server tests to get things rolling, > unless I'm missing something. Sounds great. > I think we could avoid merging D100196 <https://reviews.llvm.org/D100196> if > I split D100208 <https://reviews.llvm.org/D100208> into two parts, the > earlier part just calling `NewSubprocess()` without actually reporting stop. > This won't be really functional (or used in real sessions) but should suffice > for testing. I'm not sure how would that work -- you'd still have to provide a way to notify the client (test) about the new process and its pid, so that the test can assert something meaningful. Let's just keep D100196 <https://reviews.llvm.org/D100196>, but leave out the client specific parts -- just keep it about adding the new enums and relevant server code (trivial case switches are fine). > To be honest, I really like to keep these patches small, even if it means it > takes 2 or 3 patches to make a test. I would prefer just adding the test to > the last patch in series. I don't mind the multiple patches. These are a bit on the smaller side, but they are definitely easier to review than a single giant patch). It does create confusion though, since that is not the usual mode of operation around here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100191/new/ https://reviews.llvm.org/D100191 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
