Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added node16 execution handler #3861

Merged
merged 18 commits into from
Jul 6, 2022

Conversation

AndreyIvanov42
Copy link
Contributor

@AndreyIvanov42 AndreyIvanov42 commented Jun 10, 2022

Task description:

Added node16 execution handler - to be able to execute tasks using this version.
Mostly similar to #3103

Changelog

  • Added new execution handler
  • Changed logic for choosing node version by priority
  • Added unit tests

Testing

  • Changes have been tested on Linux, MacOS, Windows:
    by unit-tests
    manually as part of some task. Used tasks and cases are described here
  • Enough test coverage for changes and current test coverage for the task doesn't look poor
  • I didn't find any breaking changes
  • There are no other concerns
  • I haven't discovered any new uncovered test/use cases

@AndreyIvanov42 AndreyIvanov42 requested a review from a team June 14, 2022 14:31
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at the comments

src/Agent.Worker/Handlers/NodeHandler.cs Show resolved Hide resolved
src/Misc/externals.sh Outdated Show resolved Hide resolved
src/Agent.Worker/Handlers/NodeHandler.cs Outdated Show resolved Hide resolved
@alexander-smolyakov alexander-smolyakov marked this pull request as ready for review June 23, 2022 21:58
@alexander-smolyakov
Copy link
Contributor

Could you please update the PR description as well?

@AndreyIvanov42
Copy link
Contributor Author

Could you please update the PR description as well?

Done

@AndreyIvanov42
Copy link
Contributor Author

Migrated task

CopyFiles, UsePythonVersion, CMake

Environment used (OS/Agent)

Windows 10x64 / Agent with Node 16 handler, Ubuntu 16.04x64 / Agent with Node 16 handler, macOS 12 / Agent with Node 16 handler

Test cases

- task: CMake@1
  inputs:
    workingDirectory: 'src'
    cmakeArgs: -DCMAKE_SHELL=false -DTEST_ONE=one;two -DTEST_TWO=/one/two/three -DTEST_WIN_VARIABLE=test -DTEST_VARIABLE=test
    runInsideShell: false 
- task: CMake@1
  inputs:
    workingDirectory: 'src'
    cmakeArgs: -DCMAKE_SHELL=true -DTEST_ONE=one;two -DTEST_TWO=/one/two/three -DTEST_VARIABLE=$USER -DTEST_WIN_VARIABLE=%OS%
    runInsideShell: true
- task: UsePythonVersion@0
  inputs:
    versionSpec: 3.10.5
    disableDownloadFromRegistry: true
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents: *.js
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: false
    OverWrite: false
    flattenFolders: false
    preserveTimestamp: true
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents: *.js
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: false
    OverWrite: true
    flattenFolders: false
    preserveTimestamp: true
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents: *
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: true
    OverWrite: false
    flattenFolders: false
    preserveTimestamp: false
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents: **/bin/**
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: true
    OverWrite: false
    flattenFolders: true
    preserveTimestamp: false
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents: **
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: true
    OverWrite: false
    flattenFolders: true
    preserveTimestamp: false
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents: |
      *.css
      **/bin/**.css
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: true
    OverWrite: false
    flattenFolders: true
    preserveTimestamp: false
- task: CopyFiles@2
  inputs:
    SourceFolder: 'src/copy-files/source'
    Contents:  root-file1.js
    TargetFolder: 'src/copy-files/target'
    CleanTargetFolder: true
    OverWrite: false
    flattenFolders: false
    preserveTimestamp: true

src/Misc/externals.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and please take a look at the comment below

src/Agent.Worker/Handlers/NodeHandler.cs Show resolved Hide resolved
@AndreyIvanov42 AndreyIvanov42 merged commit bc8c5a1 into master Jul 6, 2022
alexander-smolyakov added a commit that referenced this pull request Jul 11, 2022
* Added node18 execution handler

* Added node18 execution handler

* Reworked tests

* Fixed typo

* Fixed typo in tests

* Fixed node path selection logic

* change to node16

* added knob for Node16

* corrected according to review

* improved log tracing

* improve code

* update version

Co-authored-by: Anatolii Bolshakov (Akvelon INC) <v-anbols@microsoft.com>
Co-authored-by: Andrey Ivanov <v-andivanov@microsoft.com>
Co-authored-by: Alexander Smolyakov <sphinx414@gmail.com>
alexander-smolyakov added a commit that referenced this pull request Jul 12, 2022
…d folders from Node distribution (#3899)

* Added node16 execution handler (#3861)
* remove npx in the node 10 directory; remove npm, npx, corepack, and related node modules in the node 16 directory (#3897)
* Update dotnet scripts (#3895)
* Agent Release 2.206.1

Co-authored-by: AndreyIvanov42 <93121155+AndreyIvanov42@users.noreply.github.com>
Co-authored-by: Anatolii Bolshakov (Akvelon INC) <v-anbols@microsoft.com>
Co-authored-by: Andrey Ivanov <v-andivanov@microsoft.com>
Co-authored-by: Denis Rumyantsev <mr.denis.rumyantsev@gmail.com>
Co-authored-by: Lilia Sabitova <lilia.sabitova@ua.akvelon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants