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

test: add test environment to run tests against local solana #93

Merged
merged 20 commits into from
Dec 28, 2021

Conversation

olgkv
Copy link
Contributor

@olgkv olgkv commented Dec 7, 2021

  • Add test environment to run tests against local solana network, using solana cli tool: solana-test-validator
  • Add External Price Account test, which is run against local solana network
  • Add Github Actions, which are allow to run tests after pull request

@olgkv olgkv requested review from kurpav and zaxozhu December 7, 2021 13:25
test/utils.ts Outdated Show resolved Hide resolved
@kurpav
Copy link
Contributor

kurpav commented Dec 7, 2021

@olgkv Good job 👍
Please add a description to this PR

@olgkv olgkv requested a review from b2kdaman December 7, 2021 15:26
Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Requested a few changes via a call.

@thlorenz
Copy link
Contributor

thlorenz commented Dec 9, 2021

@olgkv have a look at amman, a package I just created to aid in setting up the validator and which will include convenience actions to be used for testing in the future.

All you have to do is place a .ammanrc.js config in the root of the project and then run amman validator.

The config would be a combination of the below two (I spit it into base and inheritor since the mpl repo has multiple packages).

Base .ammanrc.js

// @ts-check
'use strict';
const path = require('path');

const localDeployDir = path.join(__dirname, 'target', 'deploy');

const programIds = {
  metadata: 'metaqbxxUerdq28cj1RbAWkYQm3ybzjb6a8bt518x1s',
  vault: 'vau1zxA2LbssAUEF7Gpw91zMM1LvXrvpzJtmZ58rPsn',
  auction: 'auctxRXPeJoc4817jDhf4HbjnhEcr1cCXenosMhK5R8',
  metaplex: 'p1exdMJcjVao65QdewkaZRUnU6VPSXhus9n2GzWfh98',
};

function localDeployPath(programName) {
  return path.join(localDeployDir, `${programName}.so`);
}
const programs = {
  metadata: { programId: programIds.metadata, deployPath: localDeployPath('mpl_token_metadata') },
  vault: { programId: programIds.vault, deployPath: localDeployPath('mpl_token_vault') },
  auction: { programId: programIds.auction, deployPath: localDeployPath('mpl_auction') },
  metaplex: { programId: programIds.mpl, deployPath: localDeployPath('mpl_metaplex') },
};

const validator = {
  verifyFees: true,
};

module.exports = {
  programs,
  validator,
};

TokenMetadata .ammanrc.js

'use strict';
// @ts-check
const base = require('../../.ammanrc.js');

const validator = { ...base.validator, programs: [base.programs.metadata] };
module.exports = { validator };

This also writes the validator config to the /tmp folder and cleans it up afterwards, so you don't need that empty config folder anymore either.

LMK if you need help setting this up.

Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Found some issues especially regarding the build setup depending on external tools.
Please fix and LMK when done so I can review again.

Also please DO address the points I'm raising before asking for another review, I feel like I'm commenting on some things (like the solana-validator.yml in the .gitignore) a second time.

package.json Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/utils.ts Show resolved Hide resolved
test/utils.ts Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Getting closer, please fix the final points/nits I raised.

test/setup/build.ts Outdated Show resolved Hide resolved
test/utils.ts Outdated Show resolved Hide resolved
@thlorenz
Copy link
Contributor

thlorenz commented Dec 17, 2021

You resolved the issues I had, but I'm confused WRT to the github workflow .. it doesn't seem to complete .. ever?

Pull Request Expected — Waiting for status to be reported

The changes are fine, but I don't want yo to approve/merge this until we figure out why those checks aren't running properly.

Nothing should get merged without passing checks and this PR adds a github workflow that runs integration tests as part of a workflow. So we need to see the existing ones passing before merging changes that add more.

Does anyone know why they're stuck or why they don't get triggered?

@olgkv olgkv dismissed stale reviews from zaxozhu and b2kdaman via 32eff7e December 17, 2021 18:01
Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Nice, I see the integration tests now actually are running 😄 .
There are still some issues that I pointed out.

Also could someone please figure out why the Pull Request workflow isn't running? We don't know for instance if the lint check passes and so on. It most likely also would have caught the missing TypeScript types etc.

test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/utils.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
test/setup/build.ts Outdated Show resolved Hide resolved
@olgkv olgkv requested a review from kurpav December 21, 2021 09:51
test/setup/build.ts Outdated Show resolved Hide resolved
test/metadata.test.ts Show resolved Hide resolved
Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Approving with the understanding that the skipped test will be addressed.

@kurpav kurpav merged commit 3ca7ded into main Dec 28, 2021
@kurpav kurpav deleted the test/local_solana branch December 28, 2021 04:18
@zaxozhu
Copy link
Contributor

zaxozhu commented Jan 5, 2022

🎉 This PR is included in version 4.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants