tfiala marked 4 inline comments as done.
================
Comment at: packages/Python/lldbsuite/test/dotest.py:725
@@ +724,3 @@
+ test_filename = os.path.abspath(os.path.join(dir, name))
+ if test_filename.endswith(".pyc"):
+ test_filename = test_filename[0:-1]
----------------
tfiala wrote:
> labath wrote:
> > Will this ever be executed? I mean, if we get a pyc file, we shouldn't even
> > get here due to the check on line 689.
> Ah yes, that is true. Since we assert downstream from here if it was a .pyc
> file, I was being safe.
>
> This will get refactored per previous comment, and at that point I will need
> it.
You were right, once I refactored I saw I didn't need this check. I pulled it
out.
================
Comment at: packages/Python/lldbsuite/test_event/formatter/pickled.py:57
@@ +56,3 @@
+ # end.
+ if self.use_send:
+ # Send it as
{serialized_length_of_serialized_bytes}{serialized_bytes}
----------------
labath wrote:
> tfiala wrote:
> > tfiala wrote:
> > > labath wrote:
> > > > Why do we need the format to be different based on the kind of object
> > > > we are writing to? The added magic (introspection) seems like a bad
> > > > tradeoff, if all it does is avoid a couple of lines in
> > > > `event_collector.py`
> > > When we're using sockets, we have to be able to know the size of the full
> > > info when reconstructing on the receiving side. This is the normal mode
> > > in which this is used. However, that also complicates the parsing of the
> > > data for the simple test driver.
> > >
> > > The code later on in the test_event unit tests:
> > >
> > > ```
> > > if os.path.exists(pickled_events_filename()):
> > > with open(pickled_events_filename(), "rb") as events_file:
> > > while True:
> > > try:
> > > # print("reading event")
> > > event = cPickle.load(events_file)
> > > # print("read event: {}".format(event))
> > > if event:
> > > events.append(event)
> > > except EOFError:
> > > # This is okay.
> > > Break
> > > ```
> > >
> > > Would get considerably more complicated to deal with the same format that
> > > is only required for going over a network-style protocol. I prefer this
> > > tradeoff. In the unit tests, I just send the event output to a file, and
> > > then read it with the simple loop I included above.
> > >
> > > However, to verify that I really prefer it, I will try writing it the
> > > other way.
> > > However, to verify that I really prefer it, I will try writing it the
> > > other way.
> >
> > The flip side is I can try to factor out the listener side logic that
> > reconstructs these. However, that is currently rather tightly coupled to
> > the network listening transport. And most of the work it does has purely
> > to do with needing to receive the whole message before it can try to
> > un-pickle it. So I'm not really seeing that as a big win. (Except for
> > testability. So maybe it's okay to break that out.) I'll see what that
> > looks like since that is probably the better high-level way to handle this
> > if we didn't want the change I made to the sender side.
> The other alternative I see is to make pass in a "serializer" object (or a
> lambda or something), which knows how to write to the right output. Then, you
> can construct the correct serializer object depending on whether you got
> passed `--results-file` or `--results-port`.
Still outstanding.
http://reviews.llvm.org/D20193
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
