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

[feat] cargo axiom e2e test #1003

Merged
merged 4 commits into from
Dec 12, 2024
Merged

[feat] cargo axiom e2e test #1003

merged 4 commits into from
Dec 12, 2024

Conversation

Golovanov399
Copy link
Contributor

@Golovanov399 Golovanov399 commented Dec 11, 2024

Hopefully resolves INT-2857.

It is now able to read the transpiler from the app config toml (in order to do this, I changed the (read|write)_exe_(from|to)_file functions so that the don't use any form of JSON since our maps have keys incompatible with json). Also, there is a cli e2e test now (runs on push to main (I hope) and when run-cli-e2e label is on).


Copilot:
This pull request introduces a new GitHub Actions workflow for end-to-end testing of the CLI and refactors several functions to improve file handling and transpiler configuration in the axvm-sdk and cargo-axiom crates. The most important changes are:

New GitHub Actions Workflow:

  • .github/workflows/cli-e2e.yml: Added a new workflow for running end-to-end tests on the CLI, triggered by pushes to the main branch and pull requests affecting specific paths. This includes setting up the environment, installing dependencies, and running the tests.

Refactoring for File Handling:

  • crates/axvm-sdk/src/fs.rs: Refactored the read_exe_from_file and write_exe_to_file functions to use bincode for encoding and decoding, replacing the previous BSON-based implementations.

Transpiler Configuration:

  • crates/cargo-axiom/src/commands/build.rs: Added support for specifying a transpiler configuration file via a new command-line argument --transpiler-config. This allows users to customize the transpiler extensions used during the build process. [1] [2]

Code Cleanup:

@Golovanov399 Golovanov399 added the run-cli-e2e Run a sequence of `cargo axiom *` stuff on the example program label Dec 11, 2024
Copy link

linear bot commented Dec 11, 2024

INT-2857 CLI fixes and e2e test

  • InitCmd is weird, it was hard to tell what this is. It should be part of Keygen but with a --evm flag decided to leave as is?
  • build is only going to create the RISC-V ELF because we want it to work for libraries and be the same as cargo. Therefore no one is doing the transpile step right now.
    • There is a --transpile flag
  • build --transpile --something needs to support reading from toml for Transpiler transpile
  • Finish the e2e test so we actually catch the whole pattern (the test doesn't need to run by default in CI)

@@ -15,11 +16,15 @@ use crate::{
};

pub fn read_exe_from_file<P: AsRef<Path>>(path: P) -> Result<AxVmExe<F>> {
read_from_file_bson(path)
let data = std::fs::read(path)?;
let exe = bincode::serde::decode_from_slice(&data, bincode::config::standard())?.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I will look into if we can use one package (maybe bitcode) for everything. OK for now

Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonathanpwang jonathanpwang merged commit 6256aa3 into main Dec 12, 2024
4 checks passed
@jonathanpwang jonathanpwang deleted the feat/cargo-axiom-e2e-test branch December 12, 2024 01:33
luffykai pushed a commit that referenced this pull request Dec 13, 2024
* Add transpiler config read

* Fix exe rw

* Attempt at adding a workflow

* Change the test so that it's now not a workflow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-cli-e2e Run a sequence of `cargo axiom *` stuff on the example program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants