add todo item back

Separation of concerns. We want to completely separate the internals of
Octokit from this application, so move any reference/call to Octokit into
its own file to symbolize that.

src/functions.ts

@@ -1,41 +1,15 @@
 import * as core from "@actions/core"
-import { GitHub } from "@actions/github/lib/utils"
 import * as tc from "@actions/tool-cache"
 import retry from "async-retry"
 import * as fs from "fs"
 import semver from "semver"
 
-const NEXTFLOW_REPO = { owner: "nextflow-io", repo: "nextflow" }
+import { NextflowRelease } from "./nextflow-release"
 
-// HACK Private but I want to test this
-export async function all_nf_releases(
-  ok: InstanceType<typeof GitHub>
-): Promise<object[]> {
-  return await ok.paginate(
-    ok.rest.repos.listReleases,
-    NEXTFLOW_REPO,
-    response => response.data
-  )
-}
-
-// HACK Private but I want to test this
-export async function latest_stable_release_data(
-  ok: InstanceType<typeof GitHub>
-): Promise<object> {
-  const { data: stable_release } = await ok.rest.repos.getLatestRelease(
-    NEXTFLOW_REPO
-  )
-
-  return stable_release
-}
-
-export async function release_data(
-  version: string,
-  ok: InstanceType<typeof GitHub>
-): Promise<object> {
+function tag_filter(version: string): (r: NextflowRelease) => Boolean {
   // Setup tag-based filtering
-  let filter = (r: object): boolean => {
-    return semver.satisfies(r["tag_name"], version, true)
+  let filter = (r: NextflowRelease): boolean => {
+    return semver.satisfies(r.versionNumber, version, true)
   }
 
   // Check if the user passed a 'latest*' tag, and override filtering
@@ -44,52 +18,45 @@ export async function release_data(
     if (version.includes("-everything")) {
       // No filtering
       // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      filter = (r: object) => {
+      filter = (r: NextflowRelease) => {
         return true
       }
     } else if (version.includes("-edge")) {
-      filter = r => {
-        return r["tag_name"].endsWith("-edge")
+      filter = (r: NextflowRelease) => {
+        return r.versionNumber.endsWith("-edge")
       }
     } else {
-      // This is special: passing 'latest' or 'latest-stable' allows us to use
-      // the latest stable GitHub release direct from the API
-      const stable_release = await latest_stable_release_data(ok)
-      return stable_release
+      filter = (r: NextflowRelease) => {
+        return !r.isEdge
+      }
     }
   }
+  return filter
+}
 
-  // Get all the releases
-  const all_releases: object[] = await all_nf_releases(ok)
-
-  const matching_releases = all_releases.filter(filter)
+export async function get_nextflow_release(
+  version: string,
+  releases: NextflowRelease[]
+): Promise<NextflowRelease> {
+  // Filter the releases
+  const filter = tag_filter(version)
+  const matching_releases = releases.filter(filter)
 
   matching_releases.sort((x, y) => {
     // HACK IDK why the value flip is necessary with the return
-    return semver.compare(x["tag_name"], y["tag_name"], true) * -1
+    return semver.compare(x.versionNumber, y.versionNumber, true) * -1
   })
 
   return matching_releases[0]
 }
 
-export function nextflow_bin_url(release: object, get_all: boolean): string {
-  const release_assets = release["assets"]
-  const all_asset = release_assets.filter((a: object) => {
-    return a["browser_download_url"].endsWith("-all")
-  })[0]
-  const regular_asset = release_assets.filter((a: object) => {
-    return a["name"] === "nextflow"
-  })[0]
-
-  const dl_asset = get_all ? all_asset : regular_asset
-
-  return dl_asset.browser_download_url
-}
-
 export async function install_nextflow(
-  url: string,
-  version: string
+  release: NextflowRelease,
+  get_all: boolean
 ): Promise<string> {
+  const url = get_all ? release.allBinaryURL : release.binaryURL
+  const version = release.versionNumber
+
   core.debug(`Downloading Nextflow from ${url}`)
   const nf_dl_path = await retry(
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -119,6 +86,11 @@ export async function install_nextflow(
 }
 
 export function check_cache(version: string): boolean {
+  // A 'latest*' version indicates that a cached version would be invalid until
+  // the version is resolved: abort
+  if (version.includes("latest")) {
+    return false
+  }
   const cleaned_version = semver.clean(version, true)
   if (cleaned_version === null) {
     return false

src/main.ts

@@ -1,20 +1,20 @@
 import * as core from "@actions/core"
 import * as exec from "@actions/exec"
-import * as github from "@actions/github"
-import { GitHub } from "@actions/github/lib/utils"
 import * as tc from "@actions/tool-cache"
 import * as fs from "fs"
 import semver from "semver"
 
 import {
   check_cache,
-  install_nextflow,
-  nextflow_bin_url,
-  release_data
+  get_nextflow_release,
+  install_nextflow
 } from "./functions"
+import { NextflowRelease } from "./nextflow-release"
+import { pull_releases, setup_octokit } from "./octokit-wrapper"
 
 async function run(): Promise<void> {
-  // Set environment variables
+  // CAPSULE_LOG leads to a bunch of boilerplate being output to the logs: turn
+  // it off
   core.exportVariable("CAPSULE_LOG", "none")
 
   // Read in the arguments
@@ -28,25 +28,16 @@ async function run(): Promise<void> {
   }
 
   // Setup the API
-  let octokit: InstanceType<typeof GitHub> | undefined
-  try {
-    octokit = github.getOctokit(token)
-  } catch (e: unknown) {
-    if (e instanceof Error) {
-      core.setFailed(
-        `Could not authenticate to GitHub Releases API with provided token\n${e.message}`
-      )
-    }
-  }
+  const octokit = await setup_octokit(token)
+
+  const releases = await pull_releases(octokit)
 
   // Get the release info for the desired release
-  let release = {}
+  let release = {} as NextflowRelease
   let resolved_version = ""
   try {
-    if (octokit !== undefined) {
-      release = await release_data(version, octokit)
-    }
-    resolved_version = release["tag_name"]
+    release = await get_nextflow_release(version, releases)
+    resolved_version = release.versionNumber
     core.info(
       `Input version '${version}' resolved to Nextflow ${release["name"]}`
     )
@@ -58,20 +49,10 @@ async function run(): Promise<void> {
     }
   }
 
-  // Get the download url for the desired release
-  let url = ""
-  try {
-    url = nextflow_bin_url(release, get_all)
-    core.info(`Preparing to download from ${url}`)
-  } catch (e: unknown) {
-    if (e instanceof Error) {
-      core.setFailed(`Could not parse the download URL\n${e.message}`)
-    }
-  }
   try {
     // Download Nextflow and add it to path
     if (!check_cache(resolved_version)) {
-      const nf_install_path = await install_nextflow(url, resolved_version)
+      const nf_install_path = await install_nextflow(release, get_all)
       const cleaned_version = String(semver.clean(resolved_version, true))
       const nf_path = await tc.cacheDir(
         nf_install_path,

src/nextflow-release.ts

@@ -0,0 +1,49 @@
+/**
+ * Houses the pertinent data that GitHub exposes for each Nextflow release
+ */
+export type NextflowRelease = {
+  versionNumber: string
+  isEdge: boolean
+  binaryURL: string
+  allBinaryURL: string
+}
+
+/**
+ * Converts the raw OctoKit data into a structured NextflowRelease
+ * @param data A "release" data struct from OctoKit
+ * @returns `data` converted into a `NextflowRelease`
+ */
+export function nextflow_release(data: object): NextflowRelease {
+  const nf_release: NextflowRelease = {
+    versionNumber: data["tag_name"],
+    isEdge: data["prerelease"],
+    binaryURL: nextflow_bin_url(data, false),
+    allBinaryURL: nextflow_bin_url(data, true)
+  }
+  return nf_release
+}
+
+/**
+ * Gets the download URL of a Nextflow binary
+ * @param release A "release" data struct from OctoKit
+ * @param get_all Whether to return the url for the "all" variant of Nextflow
+ * @returns The URL of the Nextflow binary
+ */
+export function nextflow_bin_url(release: object, get_all: boolean): string {
+  const release_assets = release["assets"]
+  const all_asset = release_assets.filter((a: object) => {
+    return a["browser_download_url"].endsWith("-all")
+  })[0]
+  const regular_asset = release_assets.filter((a: object) => {
+    return a["name"] === "nextflow"
+  })[0]
+
+  const dl_asset = get_all ? all_asset : regular_asset
+  if (dl_asset) {
+    return dl_asset.browser_download_url
+  } else {
+    // Old pre-release versions of Nextflow didn't have an "all" variant. To
+    // avoid downstream errors, substitute the regular url here.
+    return regular_asset.browser_download_url
+  }
+}

src/octokit-wrapper.ts

@@ -0,0 +1,62 @@
+import * as core from "@actions/core"
+import * as github from "@actions/github"
+import { GitHub } from "@actions/github/lib/utils"
+
+import { nextflow_release, NextflowRelease } from "./nextflow-release"
+
+const NEXTFLOW_REPO = { owner: "nextflow-io", repo: "nextflow" }
+
+export async function setup_octokit(
+  github_token: string
+): Promise<InstanceType<typeof GitHub>> {
+  let octokit = {} as InstanceType<typeof GitHub>
+  try {
+    octokit = github.getOctokit(github_token)
+  } catch (e: unknown) {
+    if (e instanceof Error) {
+      core.setFailed(
+        `Could not authenticate to GitHub Releases API with provided token\n${e.message}`
+      )
+    }
+  }
+  return octokit
+}
+
+export async function pull_releases(
+  octokit: InstanceType<typeof GitHub>
+): Promise<NextflowRelease[]> {
+  const all_release_data: object[] = await all_nf_release_data(octokit)
+  const all_releases: NextflowRelease[] = []
+  for (const data of all_release_data) {
+    all_releases.push(nextflow_release(data))
+  }
+
+  return all_releases
+}
+
+export async function all_nf_release_data(
+  ok: InstanceType<typeof GitHub>
+): Promise<object[]> {
+  return await ok.paginate(
+    ok.rest.repos.listReleases,
+    NEXTFLOW_REPO,
+    response => response.data
+  )
+}
+
+export async function latest_stable_release_data(
+  ok: InstanceType<typeof GitHub>
+): Promise<object> {
+  const { data: stable_release } = await ok.rest.repos.getLatestRelease(
+    NEXTFLOW_REPO
+  )
+
+  return stable_release
+}
+
+export async function pull_latest_stable_release(
+  ok: InstanceType<typeof GitHub>
+): Promise<NextflowRelease> {
+  const latest_release = await latest_stable_release_data(ok)
+  return nextflow_release(latest_release)
+}

test/functions.ts

@@ -1,37 +1,113 @@
-import * as github from "@actions/github"
-import { GitHub } from "@actions/github/lib/utils"
-import anyTest, { TestFn } from "ava" // eslint-disable-line import/no-unresolved
+import test from "ava" // eslint-disable-line import/no-unresolved
 
 import * as functions from "../src/functions"
-import { getReleaseTag, getToken } from "./utils"
+import { NextflowRelease } from "../src/nextflow-release"
 
-const test = anyTest as TestFn<{
-  token: string
-  octokit: InstanceType<typeof GitHub>
-}>
-
-test.before(t => {
-  const first = true
-  const current_token = getToken(first)
-  t.context = {
-    token: current_token,
-    octokit: github.getOctokit(current_token)
+// The Nextflow releases we are going to use for testing follow a regular
+// pattern: create a mock function to bootstrap some test data without repeating
+// ourselves
+function nf_release_gen(version_number: string): NextflowRelease {
+  const is_edge = version_number.endsWith("-edge")
+  const release: NextflowRelease = {
+    versionNumber: version_number,
+    isEdge: is_edge,
+    binaryURL: `https://github.com/nextflow-io/nextflow/releases/download/${version_number}/nextflow`,
+    allBinaryURL: `https://github.com/nextflow-io/nextflow/releases/download/${version_number}/nextflow-${version_number.replace(
+      "v",
+      ""
+    )}-all`
   }
-})
+  return release
+}
 
-test("all_nf_releases", async t => {
-  const result = await functions.all_nf_releases(t.context["octokit"])
-  t.is(typeof result, "object")
-})
+// A mock set of Nextflow releases
+const edge_is_newer = [
+  nf_release_gen("v23.09.1-edge"),
+  nf_release_gen("v23.04.3"),
+  nf_release_gen("v23.04.2")
+]
+const edge_is_older = [
+  nf_release_gen("v23.04.3"),
+  nf_release_gen("v23.04.2"),
+  nf_release_gen("v23.03.0-edge")
+]
 
-test("lastest_stable_release_data", async t => {
-  const result = await functions.latest_stable_release_data(
-    t.context["octokit"]
-  )
-  t.is(typeof result, "object")
-  const expected = await getReleaseTag("nextflow-io/nextflow", false)
-  t.is(result["tag_name"], expected)
-})
+/*
+  The whole reason this action exists is to handle the cases where a final
+  release is the "bleeding edge" release, rather than the "edge" release, even
+  though that's what the name would imply. Therefore, we need to test that the
+  'latest-everything' parameter can find the correct one regardless of whether
+  an "edge" release or a stable release is the true latest
+*/
+const release_filter_macro = test.macro(
+  async (
+    t,
+    input_version: string,
+    expected_version: string,
+    releases: NextflowRelease[]
+  ) => {
+    const matched_release = await functions.get_nextflow_release(
+      input_version,
+      releases
+    )
+    t.is(matched_release.versionNumber, expected_version)
+  }
+)
+test(
+  "Latest-everything install with newer edge release",
+  release_filter_macro,
+  "latest-everything",
+  "v23.09.1-edge",
+  edge_is_newer
+)
+test(
+  "Latest-everything install with older edge release",
+  release_filter_macro,
+  "latest-everything",
+  "v23.04.3",
+  edge_is_older
+)
+test(
+  "Latest-edge install with newer edge release",
+  release_filter_macro,
+  "latest-edge",
+  "v23.09.1-edge",
+  edge_is_newer
+)
+test(
+  "Latest-edge install with older edge release",
+  release_filter_macro,
+  "latest-edge",
+  "v23.03.0-edge",
+  edge_is_older
+)
+test(
+  "Latest-stable install with newer edge release",
+  release_filter_macro,
+  "latest",
+  "v23.04.3",
+  edge_is_newer
+)
+test(
+  "Latest-stable install with older edge release",
+  release_filter_macro,
+  "latest",
+  "v23.04.3",
+  edge_is_older
+)
+test(
+  "Fully versioned tag release",
+  release_filter_macro,
+  "v23.04.2",
+  "v23.04.2",
+  edge_is_newer
+)
+test(
+  "Partially versioned tag release",
+  release_filter_macro,
+  "v23.04",
+  "v23.04.3",
+  edge_is_newer
+)
 
-test.todo("nextflow_bin_url")
 test.todo("install_nextflow")

test/releasedata.ts

@@ -2,8 +2,9 @@ import * as github from "@actions/github"
 import { GitHub } from "@actions/github/lib/utils"
 import anyTest, { TestFn } from "ava" // eslint-disable-line import/no-unresolved
 
-import { release_data } from "../src/functions"
-import { getReleaseTag, getToken } from "./utils"
+import { nextflow_bin_url } from "../src/nextflow-release"
+import { all_nf_release_data } from "../src/octokit-wrapper"
+import { getToken } from "./utils"
 
 const test = anyTest as TestFn<{
   token: string
@@ -19,22 +20,22 @@ test.before(t => {
   }
 })
 
-const macro = test.macro(async (t, version: string) => {
-  let expected
-  if (version === "latest-stable") {
-    expected = await getReleaseTag("nextflow-io/nextflow", false)
-  } else if (version === "latest-edge") {
-    expected = await getReleaseTag("nextflow-io/nextflow", true)
-  } else if (version === "latest-everything") {
-    expected = await getReleaseTag("nextflow-io/nextflow", undefined)
-  } else {
-    expected = version
-  }
-  const result = await release_data(version, t.context["octokit"])
-  t.is(result["tag_name"], expected)
+const exists_macro = test.macro(async (t, object_name: string) => {
+  const all_releases = await all_nf_release_data(t.context.octokit)
+  const first_release = all_releases[0]
+  t.assert(first_release.hasOwnProperty(object_name))
 })
 
-test("hard version", macro, "v22.10.2")
-test("latest-stable", macro, "latest-stable")
-test("latest-edge", macro, "latest-edge")
-test("latest-everything", macro, "latest-everything")
+test("OctoKit returns tag", exists_macro, "tag_name")
+test("Octokit returns prerelease", exists_macro, "prerelease")
+test("Octokit returns assets", exists_macro, "assets")
+
+const binary_url_macro = test.macro(async (t, get_all: boolean) => {
+  const all_releases = await all_nf_release_data(t.context.octokit)
+  const first_release = all_releases[0]
+  const url = nextflow_bin_url(first_release, get_all)
+  t.notThrows(() => new URL(url))
+})
+
+test("Nextflow binary URL valid", binary_url_macro, false)
+test("Nextflow 'all' binary URL valid", binary_url_macro, true)