fix: address comparison for version string #1733
Conversation
| @@ -48,7 +49,7 @@ export class VariableSectionItem extends CustomTreeItem { | |||
| * @deprecated For VS Code 1.88+ this method won't be working any more | |||
| */ | |||
| async getVariableNameTooltip(): Promise<string> { | |||
| if (VariableSectionItem.versionInfo.version >= '1.88.0') { | |||
There was a problem hiding this comment.
any reason why AbstractElement needs replicate state of VSBrowser. requires managing duplicate state?
There was a problem hiding this comment.
I am not sure I am getting your point, can you please elaborate more? or maybe open a separate discussion/issue and we can discuss and solve there?
There was a problem hiding this comment.
when you init VSBrowser you copy the version over to AbstractElement but is there ever a situation where the version in AbstractElement would ever reflect differently. just seems like extra state to manage having to make sure AbstractElement reflects the actual version of vscode. Just an observation I would probably just consolidate the version under VSBrowser?
There was a problem hiding this comment.
I would need to double check, it can be some legacy from early stage of ExTester. The thing is extester and page-objects are separated packages. The VSBrowser instance storing a vscode version and driving execution is a parent which is loading during init phase appropriate set of page objects depending on VS Code version. Than the version is passed and stored also in page-objects. So I agree the version is same, but the point is to init page objects for a version of running browser.
Signed-off-by: Michael Pollind <quic_mpollind@quicinc.com>
Signed-off-by: Michael Pollind <quic_mpollind@quicinc.com>
djelinek
force-pushed
the
bugfix/improve-version-comparison
branch
from
ad9672a to
a41793f
Compare
|
djelinek
changed the title
Seems this is a known issue a semantic string comparison is not going to work for all version strings.
Before submitting your PR, please review the following checklist:
Before merging the PR:
mainbranch is green.