[Lldb-commits] [PATCH] D20193: test infra: catch bad decorators and import-time errors

[Todd Fiala via lldb-commits]

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