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

CLI option --format=json should change the STDERR logging to output JSON structured logging #421

Merged
merged 3 commits into from
Jul 28, 2022

Conversation

emmacasolin
Copy link
Contributor

@emmacasolin emmacasolin commented Jul 22, 2022

Description

When --format=json is specified on the CLI this should propagate to logger output (i.e. INFO/WARN/ERROR/DEBUG messages). With the new js-logger upgrade to 3.0.0 (MatrixAI/js-logger#14) this is now possible.

Issues Fixed

Tasks

  • 1. Upgrade js-logger to 3.0.0 (as well as for dependencies)
  • 2. Ensure we're using the formatting.jsonFormatter option for loggers when --format=json is set on the CLI

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@emmacasolin emmacasolin self-assigned this Jul 22, 2022
@emmacasolin emmacasolin added enhancement New feature or request development Standard development labels Jul 22, 2022
@ghost
Copy link

ghost commented Jul 27, 2022

👇 Click on the image for a new way to code review
  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

@emmacasolin
Copy link
Contributor Author

Should this PR only update js-logger and leave js-db for another PR?

@CMCDragonkai
Copy link
Member

Only js-logger. The js-db is in a different PR #419.

@emmacasolin emmacasolin force-pushed the feature-json-logger branch 2 times, most recently from 4205327 to 96777ad Compare July 27, 2022 07:28
@emmacasolin
Copy link
Contributor Author

This PR should now be ready for review/merge. All of the ts-ignore patches to deal with the DB still using an old version of js-logger are separated out into the most recent commit (so that it can easily be reverted once no longer needed).

@emmacasolin emmacasolin marked this pull request as ready for review July 27, 2022 07:31
@CMCDragonkai
Copy link
Member

I think you need to set the formatter in src/bin/polykey-agent.ts as well.

The formatting configuration should be propagated to the agent when it starts.

Do you have some tests for this change?

Also can you show a screenshot of it being executed and showing up JSON logs?

@CMCDragonkai
Copy link
Member

If this is merged first @tegefaulkes will there be any conflicts?

@CMCDragonkai CMCDragonkai changed the title --format=json CLI option affects logger output CLI option --format=json should change the STDERR logging to output JSON structured logging Jul 27, 2022
@emmacasolin
Copy link
Contributor Author

I think you need to set the formatter in src/bin/polykey-agent.ts as well.

The formatting configuration should be propagated to the agent when it starts.

Added this in now

Do you have some tests for this change?

Not yet, but I'll write some now

Also can you show a screenshot of it being executed and showing up JSON logs?

Sure, here's an agent starting and stopping with JSON logs

{"level":"INFO","key":"PolykeyAgent","msg":"Creating PolykeyAgent"}
{"level":"INFO","key":"PolykeyAgent","msg":"Setting umask to 077"}
{"level":"INFO","key":"PolykeyAgent","msg":"Setting node path to /tmp/a"}
{"level":"INFO","key":"Status","msg":"Starting Status"}
{"level":"INFO","key":"Status","msg":"Writing Status to /tmp/a/status.json"}
{"level":"INFO","key":"Status","msg":"Status is STARTING"}
{"level":"INFO","key":"Schema","msg":"Creating Schema"}
{"level":"INFO","key":"Schema","msg":"Starting Schema"}
{"level":"INFO","key":"Schema","msg":"Setting state path to /tmp/a/state"}
{"level":"INFO","key":"Schema","msg":"Started Schema"}
{"level":"INFO","key":"Schema","msg":"Created Schema"}
{"level":"INFO","key":"KeyManager","msg":"Creating KeyManager"}
{"level":"INFO","key":"KeyManager","msg":"Setting keys path to /tmp/a/state/keys"}
{"level":"INFO","key":"KeyManager","msg":"Starting KeyManager"}
{"level":"INFO","key":"KeyManager","msg":"Checking /tmp/a/state/keys/root.pub and /tmp/a/state/keys/root.key"}
{"level":"INFO","key":"KeyManager","msg":"Reading /tmp/a/state/keys/root.pub and /tmp/a/state/keys/root.key"}
{"level":"INFO","key":"KeyManager","msg":"Checking /tmp/a/state/keys/root.crt"}
{"level":"INFO","key":"KeyManager","msg":"Reading /tmp/a/state/keys/root.crt"}
{"level":"INFO","key":"KeyManager","msg":"Checking /tmp/a/state/keys/db.key"}
{"level":"INFO","key":"KeyManager","msg":"Reading /tmp/a/state/keys/db.key"}
{"level":"INFO","key":"KeyManager","msg":"Started KeyManager"}
{"level":"INFO","key":"KeyManager","msg":"Created KeyManager"}
{"level":"INFO","key":"DB","msg":"Creating DB"}
{"level":"INFO","key":"DB","msg":"Starting DB"}
{"level":"INFO","key":"DB","msg":"Setting DB path to /tmp/a/state/db"}
{"level":"INFO","key":"DB","msg":"Started DB"}
{"level":"INFO","key":"DB","msg":"Created DB"}
{"level":"INFO","key":"IdentitiesManager","msg":"Creating IdentitiesManager"}
{"level":"INFO","key":"IdentitiesManager","msg":"Starting IdentitiesManager"}
{"level":"INFO","key":"IdentitiesManager","msg":"Started IdentitiesManager"}
{"level":"INFO","key":"IdentitiesManager","msg":"Created IdentitiesManager"}
{"level":"INFO","key":"Sigchain","msg":"Creating Sigchain"}
{"level":"INFO","key":"Sigchain","msg":"Starting Sigchain"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 0"}
{"level":"INFO","key":"Sigchain","msg":"Started Sigchain"}
{"level":"INFO","key":"Sigchain","msg":"Created Sigchain"}
{"level":"INFO","key":"ACL","msg":"Creating ACL"}
{"level":"INFO","key":"ACL","msg":"Starting ACL"}
{"level":"INFO","key":"ACL","msg":"Started ACL"}
{"level":"INFO","key":"ACL","msg":"Created ACL"}
{"level":"INFO","key":"GestaltGraph","msg":"Creating GestaltGraph"}
{"level":"INFO","key":"GestaltGraph","msg":"Starting GestaltGraph"}
{"level":"INFO","key":"GestaltGraph","msg":"Started GestaltGraph"}
{"level":"INFO","key":"GestaltGraph","msg":"Created GestaltGraph"}
{"level":"INFO","key":"Proxy","msg":"Creating Proxy"}
{"level":"INFO","key":"Proxy","msg":"Created Proxy"}
{"level":"INFO","key":"NodeGraph","msg":"Creating NodeGraph"}
{"level":"INFO","key":"NodeGraph","msg":"Starting NodeGraph"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 1"}
{"level":"INFO","key":"NodeGraph","msg":"Started NodeGraph"}
{"level":"INFO","key":"NodeGraph","msg":"Created NodeGraph"}
{"level":"INFO","key":"NodeManager","msg":"Starting NodeManager"}
{"level":"DEBUG","key":"NodeManager","msg":"refresh bucket queue has plugged"}
{"level":"DEBUG","key":"NodeManager","msg":"Resuming refreshBucketQueue"}
{"level":"INFO","key":"NodeManager","msg":"Started NodeManager"}
{"level":"INFO","key":"Discovery","msg":"Creating Discovery"}
{"level":"INFO","key":"Discovery","msg":"Starting Discovery"}
{"level":"DEBUG","key":"Discovery","msg":"Setting up DiscoveryQueue"}
{"level":"INFO","key":"Discovery","msg":"Started Discovery"}
{"level":"INFO","key":"Discovery","msg":"Created Discovery"}
{"level":"INFO","key":"NotificationsManager","msg":"Creating NotificationsManager"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 2"}
{"level":"INFO","key":"NotificationsManager","msg":"Starting NotificationsManager"}
{"level":"DEBUG","key":"Discovery","msg":"DiscoveryQueue is pausing"}
{"level":"INFO","key":"NotificationsManager","msg":"Started NotificationsManager"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 2"}
{"level":"INFO","key":"NotificationsManager","msg":"Created NotificationsManager"}
{"level":"INFO","key":"VaultManager","msg":"Creating VaultManager"}
{"level":"INFO","key":"VaultManager","msg":"Setting vaults path to /tmp/a/state/vaults"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 3"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 3"}
{"level":"INFO","key":"VaultManager","msg":"Starting VaultManager"}
{"level":"INFO","key":"DB","msg":"Creating DB"}
{"level":"INFO","key":"DB","msg":"Starting DB"}
{"level":"INFO","key":"DB","msg":"Setting DB path to /tmp/a/state/vaults/efs"}
{"level":"INFO","key":"DB","msg":"Started DB"}
{"level":"INFO","key":"DB","msg":"Created DB"}
{"level":"INFO","key":"INodeManager","msg":"Creating INodeManager"}
{"level":"INFO","key":"INodeManager","msg":"Starting INodeManager"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 0"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 0"}
{"level":"INFO","key":"INodeManager","msg":"Started INodeManager"}
{"level":"INFO","key":"INodeManager","msg":"Created INodeManager"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 1"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 1"}
{"level":"INFO","key":"EncryptedFileSystem","msg":"Starting EncryptedFS"}
{"level":"INFO","key":"EncryptedFileSystem","msg":"Started EncryptedFS"}
{"level":"INFO","key":"VaultManager","msg":"Started VaultManager"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 3"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 3"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 3"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 3"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 3"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 3"}
{"level":"INFO","key":"VaultManager","msg":"Created VaultManager"}
{"level":"INFO","key":"SessionManager","msg":"Creating SessionManager"}
{"level":"INFO","key":"SessionManager","msg":"Starting SessionManager"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 4"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 4"}
{"level":"INFO","key":"SessionManager","msg":"Started SessionManager"}
{"level":"INFO","key":"SessionManager","msg":"Created SessionManager"}
{"level":"INFO","key":"PolykeyAgent","msg":"Starting PolykeyAgent"}
{"level":"INFO","key":"GRPCServerClient","msg":"Starting GRPCServer on 127.0.0.1:0"}
{"level":"INFO","key":"GRPCServerClient","msg":"Started GRPCServer on 127.0.0.1:35481"}
{"level":"INFO","key":"GRPCServerAgent","msg":"Starting GRPCServer on 127.0.0.1:0"}
{"level":"INFO","key":"GRPCServerAgent","msg":"Started GRPCServer on 127.0.0.1:35803"}
{"level":"INFO","key":"Proxy","msg":"Starting Forward Proxy from 127.0.0.1:0 to 0.0.0.0:0 and Reverse Proxy from 0.0.0.0:0 to 127.0.0.1:35803"}
{"level":"INFO","key":"Proxy","msg":"Started Forward Proxy from 127.0.0.1:42543 to 0.0.0.0:33780 and Reverse Proxy from 0.0.0.0:33780 to 127.0.0.1:35803"}
{"level":"INFO","key":"Queue","msg":"Starting Queue"}
{"level":"DEBUG","key":"Queue","msg":"Starting queue"}
{"level":"DEBUG","key":"Queue","msg":"Plugging queue"}
{"level":"INFO","key":"Queue","msg":"Started Queue"}
{"level":"INFO","key":"NodeConnectionManager","msg":"Starting NodeConnectionManager"}
{"level":"INFO","key":"NodeConnectionManager","msg":"Started NodeConnectionManager"}
{"level":"INFO","key":"NodeConnectionManager","msg":"Syncing nodeGraph"}
{"level":"INFO","key":"Status","msg":"Finish Status STARTING"}
{"level":"INFO","key":"Status","msg":"Writing Status to /tmp/a/status.json"}
{"level":"INFO","key":"Status","msg":"Status is LIVE"}
{"level":"INFO","key":"PolykeyAgent","msg":"Started PolykeyAgent"}
{"level":"INFO","key":"PolykeyAgent","msg":"Created PolykeyAgent"}
{"level":"INFO","key":"WorkerManager","msg":"Creating WorkerManager"}
{"level":"INFO","key":"WorkerManager","msg":"Created WorkerManager"}
{"pid":1824960,"nodeId":"vgm3r20qdo91ua7qq4tmdta4s6h5922lea4aamh0fu1373tmlevng","clientHost":"127.0.0.1","clientPort":35481,"agentHost":"127.0.0.1","agentPort":35803,"proxyHost":"0.0.0.0","proxyPort":33780,"forwardHost":"127.0.0.1","forwardPort":42543}
^C{"level":"INFO","key":"WorkerManager","msg":"Destroying WorkerManager"}
{"level":"INFO","key":"WorkerManager","msg":"Destroyed WorkerManager"}
{"level":"INFO","key":"PolykeyAgent","msg":"Stopping PolykeyAgent"}
{"level":"INFO","key":"Status","msg":"Begin Status STOPPING"}
{"level":"INFO","key":"Status","msg":"Writing Status to /tmp/a/status.json"}
{"level":"INFO","key":"Status","msg":"Status is STOPPING"}
{"level":"INFO","key":"SessionManager","msg":"Stopping SessionManager"}
{"level":"INFO","key":"SessionManager","msg":"Stopped SessionManager"}
{"level":"INFO","key":"NotificationsManager","msg":"Stopping NotificationsManager"}
{"level":"INFO","key":"NotificationsManager","msg":"Stopped NotificationsManager"}
{"level":"INFO","key":"VaultManager","msg":"Stopping VaultManager"}
{"level":"INFO","key":"EncryptedFileSystem","msg":"Stopping EncryptedFS"}
{"level":"INFO","key":"INodeManager","msg":"Stopping INodeManager"}
{"level":"DEBUG","key":"DB","msg":"Creating DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Created DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Committing DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Committed DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Finalize DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Finalized DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Destroying DBTransaction 2"}
{"level":"DEBUG","key":"DB","msg":"Destroyed DBTransaction 2"}
{"level":"INFO","key":"INodeManager","msg":"Stopped INodeManager"}
{"level":"INFO","key":"DB","msg":"Stopping DB"}
{"level":"INFO","key":"DB","msg":"Stopped DB"}
{"level":"INFO","key":"EncryptedFileSystem","msg":"Stopped EncryptedFS"}
{"level":"INFO","key":"VaultManager","msg":"Stopped VaultManager"}
{"level":"INFO","key":"Discovery","msg":"Stopping Discovery"}
{"level":"DEBUG","key":"Discovery","msg":"DiscoveryQueue is ending"}
{"level":"INFO","key":"Discovery","msg":"Stopped Discovery"}
{"level":"INFO","key":"NodeConnectionManager","msg":"Stopping NodeConnectionManager"}
{"level":"INFO","key":"NodeConnectionManager","msg":"Stopped NodeConnectionManager"}
{"level":"INFO","key":"NodeGraph","msg":"Stopping NodeGraph"}
{"level":"INFO","key":"NodeGraph","msg":"Stopped NodeGraph"}
{"level":"INFO","key":"NodeManager","msg":"Stopping NodeManager"}
{"level":"DEBUG","key":"NodeManager","msg":"refresh bucket queue has unplugged"}
{"level":"DEBUG","key":"NodeManager","msg":"Resuming refreshBucketQueue"}
{"level":"INFO","key":"NodeManager","msg":"Stopped NodeManager"}
{"level":"DEBUG","key":"NodeManager","msg":"startRefreshBucketQueue has ended"}
{"level":"INFO","key":"Queue","msg":"Stopping Queue"}
{"level":"DEBUG","key":"Queue","msg":"Stopping queue"}
{"level":"DEBUG","key":"Queue","msg":"Unplugging queue"}
{"level":"DEBUG","key":"Queue","msg":"queue has ended"}
{"level":"INFO","key":"Queue","msg":"Stopped Queue"}
{"level":"INFO","key":"Proxy","msg":"Stopping Proxy Server"}
{"level":"INFO","key":"Proxy","msg":"Stopped Proxy Server"}
{"level":"INFO","key":"GRPCServerAgent","msg":"Stopping GRPCServer"}
{"level":"INFO","key":"GRPCServerAgent","msg":"Stopped GRPCServer"}
{"level":"INFO","key":"GRPCServerClient","msg":"Stopping GRPCServer"}
{"level":"INFO","key":"GRPCServerClient","msg":"Stopped GRPCServer"}
{"level":"INFO","key":"GestaltGraph","msg":"Stopping GestaltGraph"}
{"level":"INFO","key":"GestaltGraph","msg":"Stopped GestaltGraph"}
{"level":"INFO","key":"ACL","msg":"Stopping ACL"}
{"level":"INFO","key":"ACL","msg":"Stopped ACL"}
{"level":"INFO","key":"Sigchain","msg":"Stopping Sigchain"}
{"level":"INFO","key":"Sigchain","msg":"Stopped Sigchain"}
{"level":"INFO","key":"IdentitiesManager","msg":"Stopping IdentitiesManager"}
{"level":"INFO","key":"IdentitiesManager","msg":"Stopped IdentitiesManager"}
{"level":"INFO","key":"DB","msg":"Stopping DB"}
{"level":"INFO","key":"DB","msg":"Stopped DB"}
{"level":"INFO","key":"KeyManager","msg":"Stopping KeyManager"}
{"level":"INFO","key":"KeyManager","msg":"Stopped KeyManager"}
{"level":"INFO","key":"Schema","msg":"Stopping Schema"}
{"level":"INFO","key":"Schema","msg":"Stopped Schema"}
{"level":"INFO","key":"Status","msg":"Stopping Status"}
{"level":"INFO","key":"Status","msg":"Writing Status to /tmp/a/status.json"}
{"level":"INFO","key":"Status","msg":"Status is DEAD"}
{"level":"INFO","key":"PolykeyAgent","msg":"Stopped PolykeyAgent"}

@emmacasolin emmacasolin force-pushed the feature-json-logger branch 2 times, most recently from 3651d65 to 037ef6e Compare July 28, 2022 00:13
@emmacasolin
Copy link
Contributor Author

I've rebased on staging and added a test for logger formatting. This should be good to merge now.

- Reverted Windows to NodeJS v16.14.2
- Added `--arg ci true` to `nix-shell`
- Introduced continuous benchmarking
import * as execUtils from '../utils/exec';
import { runTestIfPlatforms } from '../utils';

describe('polykey', () => {
runTestIfPlatforms('lunix', 'docker')('default help display', async () => {
runTestIfPlatforms('linux', 'docker')('default help display', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

@tegefaulkes why does this require a conditional check?

Copy link
Contributor

Choose a reason for hiding this comment

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

That should've been removed. I must've missed it.

const result = await execUtils.pkStdio([]);
expect(result.exitCode).toBe(0);
expect(result.stdout).toBe('');
expect(result.stderr.length > 0).toBe(true);
});
runTestIfPlatforms('docker')('format option affects STDERR', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, does this mean this test only runs under docker platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got a little confused by the wording but that test runs both locally and in the testing phase of the CI/CD so I think it means if the platform is docker then run it (but if the platform isn't specified also run it).

Copy link
Member

Choose a reason for hiding this comment

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

Ok I think the wording needs to be changed in a commit in staging. It should be a strict AND logic and OR logic between conditions.

This is why it shouldn't have changed to just a string list. A Boolean conditional is more obvious and flexible.

Copy link
Contributor

@tegefaulkes tegefaulkes Jul 28, 2022

Choose a reason for hiding this comment

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

It runs by default if the platform isn't specified. This is because we need to run all tests during normal testing and I'd rather not have to set PK_TEST_PLATFORM for that. I think we can make it non-implicit by using runTestIfPlatforms(undefined, 'docker')?

@CMCDragonkai
Copy link
Member

Ok merging, but I think there needs to be a change for the run condition utility functions later.

@CMCDragonkai CMCDragonkai merged commit 21aca8a into staging Jul 28, 2022
@CMCDragonkai
Copy link
Member

CMCDragonkai commented Jul 28, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants