selenium icon indicating copy to clipboard operation
selenium copied to clipboard

[py]: Fix a bug with some invalid generated python modules for devtools

Open symonk opened this issue 3 years ago • 2 comments

Highlighted in #10804 I had a quick look at the modules generated are referencing other classes inside the debugger.py by the debugging.ClassName which is not available in the name space; causing the error. This small patch handles that particular edge case; however the auto generation library we are using is a little out dated, as devtools keeps moving forward we should be wary of that (I know we wan't to technically drop devtools as quickly as possible in favour of bidi)

DebugSymbols & ScriptLanguage was added in r814213, I think we are quite out dated. I still need to test the change(s) here but the generated files are subsequently 'patched' to use the term loosely.

closes #10804

output of generation now using ../common/devtools/*.json

Now:

    @classmethod
    def from_json(cls, json: T_JSON_DICT) -> ScriptParsed:
        return cls(
            script_id=runtime.ScriptId.from_json(json['scriptId']),
            url=str(json['url']),
            start_line=int(json['startLine']),
            start_column=int(json['startColumn']),
            end_line=int(json['endLine']),
            end_column=int(json['endColumn']),
            execution_context_id=runtime.ExecutionContextId.from_json(json['executionContextId']),
            hash_=str(json['hash']),
            execution_context_aux_data=dict(json['executionContextAuxData']) if 'executionContextAuxData' in json else None,
            is_live_edit=bool(json['isLiveEdit']) if 'isLiveEdit' in json else None,
            source_map_url=str(json['sourceMapURL']) if 'sourceMapURL' in json else None,
            has_source_url=bool(json['hasSourceURL']) if 'hasSourceURL' in json else None,
            is_module=bool(json['isModule']) if 'isModule' in json else None,
            length=int(json['length']) if 'length' in json else None,
            stack_trace=runtime.StackTrace.from_json(json['stackTrace']) if 'stackTrace' in json else None,
            code_offset=int(json['codeOffset']) if 'codeOffset' in json else None,
            # culprits below:
            script_language=ScriptLanguage.from_json(json['scriptLanguage']) if 'scriptLanguage' in json else None,
            debug_symbols=DebugSymbols.from_json(json['debugSymbols']) if 'debugSymbols' in json else None
        )

vs trunk:

            code_offset=int(json['codeOffset']) if 'codeOffset' in json else None,
            # cannot reference via such a namespace:
            script_language=debugger.ScriptLanguage.from_json(json['scriptLanguage']) if 'scriptLanguage' in json else None,
            debug_symbols=debugger.DebugSymbols.from_json(json['debugSymbols']) if 'debugSymbols' in json else None
        )

symonk avatar Jun 26 '22 21:06 symonk

Codecov Report

Merging #10817 (9433b60) into trunk (20b6957) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 9433b60 differs from pull request most recent head 86aa2a8. Consider uploading reports for the commit 86aa2a8 to get more accurate results

@@           Coverage Diff           @@
##            trunk   #10817   +/-   ##
=======================================
  Coverage   50.43%   50.43%           
=======================================
  Files          84       84           
  Lines        5480     5480           
  Branches      279      279           
=======================================
  Hits         2764     2764           
  Misses       2437     2437           
  Partials      279      279           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Jun 26 '22 21:06 codecov-commenter

@AutomatedTester / @titusfortner I think this needs a wider discussion maybe; we are using a library which generates things and it is not actively maintained since circa 2020; there is a fork of said repo. I suspect a fair bit of this doesn't really work properly and this is only fixing a single case.

Is my understanding from digging into how it works, cdp has likely drastically moved on from what we are generating and more issues are likely to arise in future.

symonk avatar Aug 12 '22 10:08 symonk