Use consistent CI tooling. Include recent `local` single-repo feature. #1

joewagner · 2023-07-18T17:10:25Z

Overview

This PR brings this repo up to date with the recent singular package repo feature additions, and also includes changes to linting and testing.

Details

The linting and testing changes are significant because each of the packages had slight variations on how that tooling was setup. The majority of the changes were bringing local and cli up to date with sdk

joewagner · 2023-08-07T20:50:39Z

@carsonfarmer There's a ton of changes here, as such it's potentially better to just look at the state of the branch then look at the diff.
Everything works as expected for me locally, but unfortunately the test check just hangs forever. I'm going to try to setup a local github action runner and see if I can gain some insight into why the tests are hanging.
If you get a free cycle, it would be great to get feedback on how all of this is setup.

joewagner · 2023-08-08T00:08:19Z

UPDATE:
I had to remove the SDK's playwright browser tests, but all tests are now passing (and even running in parallel). I'll take this out of draft aand if everything looks good we can start using this repo for releases.

joewagner · 2023-08-08T15:45:38Z

packages/cli/test/setup.ts

 export const TEST_TIMEOUT_FACTOR = getTimeoutFactor();
+export const TEST_PROVIDER_URL = `http://127.0.0.1:${registryPort}`;
+export const TEST_VALIDATOR_URL = "http://localhost:8082/api/v1";


Using custom ports for each package so tests can run in parallel

joewagner · 2023-08-08T15:46:42Z

packages/cli/test/shell.test.ts


-  const accounts = getAccounts();
-  const db = getDatabase(accounts[1]);


Because of the use of custom ports we can't use the default database connection anymore.

joewagner · 2023-08-08T15:48:32Z

packages/cli/test/shell.test.ts

+  TEST_PROVIDER_URL,
+  "--chain",
+  "local-tableland",
+];


The --chain flag was getting repeated for most of the tests, plus we need a way to specify the provider url that includes the custom port.

joewagner · 2023-08-08T16:34:44Z

packages/cli/test/validator/config.json

+{
+  "Impl": "mesa",
+  "HTTP": {
+    "Port": "8082",


Each package's tests have their own validator config, which enables Lerna to run in parallel.

joewagner · 2023-08-08T16:36:34Z

packages/cli/test/write.test.ts


    equal(resolverMock.calledOnce, true);
  });
+
+  test("passes with GRANT statement", async function () {


These tests are an update that came from change to the singular repo in the last few weeks.

joewagner · 2023-08-08T16:51:20Z

packages/local/src/main.ts

@@ -175,67 +220,97 @@ class LocalTableland {
    console.log("\n\n*************************************\n");
  }

-  async #_setReady() {
+  async #_setReady(): Promise<void> {


The local tests were not written in typescript. As such there are a lot of changes that get typescript linting passing.

joewagner · 2023-08-08T16:52:30Z

packages/local/src/up.ts

+    description: "Path the the Tableland Registry contract repository.",
+  },
+  docker: {
+    type: "boolean",


added in tablelandnetwork/local-tableland#423

joewagner · 2023-08-08T16:55:50Z

packages/sdk/package.json

    "prepublishOnly": "npm run build",
-    "test": "mocha",
-    "test:browser": "PW_TS_ESM_ON=true playwright test",
+    "test": "npm run test:browser && mocha --exit",


I'm not entirely sure why, but the github test workflow hangs forever if playwright tests are included. It may have to do with the custom ports for test networks, or some issue downloading the playwright test browsers. Either way, I wanted to get the transition to a monorepo done before looking into it anymore, so I've removed the requirement for browser tests from CI.

Ok, yeh good idea. We can sort that out later.

joewagner · 2023-08-08T16:57:31Z

packages/sdk/src/database.ts

+      // re-normalize so we can be sure we've isolated each statement and its tableId
+      const norm = await normalize(stmt);
+
+      if (norm.tables.length > 1) {


This is from tablelandnetwork/js-tableland#562

I know this is from a previous PR, but we might want to explore handling more of this in the parser?

joewagner · 2023-08-08T17:05:34Z

packages/sdk/src/helpers/config.ts

+
+export function jsonFileAliases(filepath: string): AliasesNameMap {
+  return {
+    read: async function (): Promise<NameMapping> {


from tablelandnetwork/js-tableland#542

joewagner · 2023-08-11T16:43:49Z

@asutula I tagged you as a reviewer here. @carsonfarmer probably has the most context to do a review, but if this kind of setup works well I'd suggest that we setup studio in a similar way. Specifically, the app could be a package and the cli could be a package.

joewagner · 2023-08-18T18:08:49Z

@carsonfarmer This PR is kind of a lot of changes, is there anything I can do to make it easier to review?

carsonfarmer

Ok, wow. This was a lot of work @joewagner! Really great stuff. Let's get this merged in, and we'll move forward from a unified point.

Really great work here, just a few tiny nits. We'll likely fine a few issues once we get this merged as well... so we can create separate issues for this as they crop up.

carsonfarmer · 2023-08-21T17:16:28Z

.github/CODEOWNERS

@@ -2,4 +2,7 @@
 # the repo. Unless a later match takes precedence.
 # By convention in @tablelandnetwork repos, the first listed
 # CODEOWNER is the repo DRI (directly responsible individual)
-*       @joewagner @carsonfarmer @awmuncy
+* @carsonfarmer @joewagner
+* /packages/sdk/ @carsonfarmer @joewagner


Remove the *s on these lines

carsonfarmer · 2023-08-21T17:49:45Z

packages/local/src/main.ts

+      // process was already killed—it *is* possible with the validator process
+      // but doesn't seem to happen with the Registry
+      try {
+        // @ts-expect-error pic is possibly undefined, which is fine


should say pid

carsonfarmer · 2023-08-21T17:55:35Z

packages/sdk/package.json

    "prepublishOnly": "npm run build",
-    "test": "mocha",
-    "test:browser": "PW_TS_ESM_ON=true playwright test",
+    "test": "npm run test:browser && mocha --exit",


Ok, yeh good idea. We can sort that out later.

carsonfarmer · 2023-08-21T18:04:03Z

packages/sdk/src/database.ts

+      // re-normalize so we can be sure we've isolated each statement and its tableId
+      const norm = await normalize(stmt);
+
+      if (norm.tables.length > 1) {


I know this is from a previous PR, but we might want to explore handling more of this in the parser?

carsonfarmer

👍

joewagner added 13 commits July 14, 2023 17:02

Update the sdk with latest in old repo, add nx

6a87fed

Add newest cli

aa511f5

combine prettier and git ignore files into one

3093e37

global linting for sdk and cli

79b82ee

Update local with latest from old repo

868a61e

update cli with old repo

4e2f168

collecting more lint rules from old repos

0016829

de-duplicate github directory

3b0d8c5

fix dependabot

e0022ad

improve test workflow

5577804

update sdk

47edf48

ensure browser test deps get installed

b4198bc

bump validator and fix local test:ci command

ab188fb

joewagner force-pushed the joe/init-setup branch from c015684 to ab188fb Compare July 21, 2023 15:14

joewagner added 16 commits July 21, 2023 12:03

correct target deps + force ignore test only deps

637a662

setup playwright install before ci test

ade2aa4

update gitignore and silence l.t.

d89f357

wip enabling parallel test running

3ce3b04

sync all packages with linting build etc...

ed7e203

updating mocha config file

fe8b74a

add types for local, include test helper for local

9473bef

ignore sdk test db files

bce4d65

pretty fixes

c4e2cc6

update lint rules

809d371

Add test helper

773893e

fix sdk test network port setup

8729eb8

pretty fixes

e42dfdc

ignore dist in coverage tests

c6c5a10

use non-standard ports for cli tests network

5fb2e7a

cleanup cli test and fix coverages

c7432bc

turn on full logging

b04e598

joewagner added 4 commits August 7, 2023 15:57

Add ssh debugger and ensure mocha exits

8abb63d

debugging: try ci tests without playwright

12cb45c

ensure local builds before tests and turn off logs

cb5a208

Fix incorrect removal of get-dirname.ts in local

b9155b9

joewagner marked this pull request as ready for review August 8, 2023 00:08

joewagner requested a review from carsonfarmer August 8, 2023 00:08

joewagner changed the title ~~Working on cleaning up and re-using CI and tooling~~ Use consistent CI tooling. Include recent local single-repo feature. Aug 8, 2023

joewagner commented Aug 8, 2023

View reviewed changes

joewagner requested a review from asutula August 11, 2023 16:41

carsonfarmer previously approved these changes Aug 21, 2023

View reviewed changes

joewagner dismissed carsonfarmer’s stale review via 97d5cc6 August 21, 2023 18:13

Fix codeowners and typo

1cac967

joewagner force-pushed the joe/init-setup branch from 97d5cc6 to 1cac967 Compare August 21, 2023 18:14

joewagner requested a review from carsonfarmer August 21, 2023 18:17

carsonfarmer approved these changes Aug 21, 2023

View reviewed changes

joewagner merged commit f123ece into main Aug 21, 2023
4 checks passed

joewagner deleted the joe/init-setup branch August 21, 2023 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use consistent CI tooling. Include recent `local` single-repo feature. #1

Use consistent CI tooling. Include recent `local` single-repo feature. #1

joewagner commented Jul 18, 2023 •

edited

Loading

joewagner commented Aug 7, 2023

joewagner commented Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

joewagner Aug 8, 2023

carsonfarmer Aug 21, 2023

joewagner Aug 8, 2023

carsonfarmer Aug 21, 2023

joewagner Aug 21, 2023

joewagner Aug 8, 2023

joewagner commented Aug 11, 2023

joewagner commented Aug 18, 2023

carsonfarmer left a comment

carsonfarmer Aug 21, 2023

carsonfarmer Aug 21, 2023

carsonfarmer Aug 21, 2023

carsonfarmer Aug 21, 2023

carsonfarmer left a comment


		const accounts = getAccounts();
		const db = getDatabase(accounts[1]);

Use consistent CI tooling. Include recent local single-repo feature. #1

Use consistent CI tooling. Include recent local single-repo feature. #1

Conversation

joewagner commented Jul 18, 2023 • edited Loading

Overview

Details

joewagner commented Aug 7, 2023

joewagner commented Aug 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joewagner commented Aug 11, 2023

joewagner commented Aug 18, 2023

carsonfarmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carsonfarmer left a comment

Choose a reason for hiding this comment

Use consistent CI tooling. Include recent `local` single-repo feature. #1

Use consistent CI tooling. Include recent `local` single-repo feature. #1

joewagner commented Jul 18, 2023 •

edited

Loading