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

Install go from releases #1

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dmitry-shibanov
Copy link
Owner

@dmitry-shibanov dmitry-shibanov commented Jun 22, 2020

Since added go-versions was added to actions, we need add logic to install go from our releases.

  • added installation go from releases
  • added go tests

@MaksimZhukov
Copy link
Collaborator

Why do we need the __tests__/go_release.json file?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@dmitry-shibanov dmitry-shibanov force-pushed the v-dmshib/update-for-release branch 3 times, most recently from 9a71e4e to 3632aa4 Compare June 22, 2020 20:51
src/installer.ts Outdated Show resolved Hide resolved
src/installer.ts Outdated Show resolved Hide resolved
Comment on lines 2 to 6
on:
push:
pull_request:
schedule:
- cron: 0 0 * * *
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest to update the event configuration in the final version of yml:

  push:
    branches:
      - master
    paths-ignore:
      - '**.md'
  pull_request:
    paths-ignore:
      - '**.md'

{
"version": "1.2.3",
"stable": true,
"release_url": "https://github.com/actions/sometool/releases/tag/1.2.3-20200402.6",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can delete this file

expect(fileName).toBe('go1.13.7.windows-amd64.zip');
it('can mock manifest versions', async () => {
let versions: tc.IToolRelease[] | null = await tc.getManifestFromRepo(
'actions',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we don't need the following tests in the form they currently exist:

  1. it('can mock manifest versions', async () => {
  2. it('can find 1.12.17 from manifest on osx', async () => {
  3. it('can find 12 from manifest on linux', async () => {
  4. it('can find 10 from manifest on windows', async () => {

These "it" blocks test the functions located in the actions/tool-cache repository (getManifestFromRepo, findFromManifest etc.) and we don't have tests with several functions from installer.ts script (getGo, installGoVersion, extractGoArchive etc.).

@maxim-lobanov

dmitry-shibanov and others added 5 commits June 24, 2020 19:58
* add tests

* fixing tests

* change names of it

* resolve comments

Co-authored-by: Dmitry Shibanov <v-dmshib@microsoft.com>
* fix comments

* formatting

Co-authored-by: Dmitry Shibanov <v-dmshib@microsoft.com>
Co-authored-by: Dmitry Shibanov <v-dmshib@microsoft.com>
Copy link

@Lucho100789 Lucho100789 left a comment

Choose a reason for hiding this comment

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

Like

return file.arch === archFilter && file.os === platFilter;
});
if (goFile) {
core_1.debug(`matched ${candidate.version}`);
core.debug(`matched ${candidate.version}`);

Choose a reason for hiding this comment

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

Suggested change
core.debug(`matched ${candidate.version}`);
core.debug(`matched ${candidate.version}`);

@Lucho100789
Copy link

Since added go-versions was added to actions, we need add logic to install go from our releases.

  • added installation go from releases
  • added go tests

ddc7688

dmitry-shibanov pushed a commit that referenced this pull request Nov 7, 2023
Improve documentation regarding dependencies caching
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants