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

chore: refactor server into server crate #146

Closed
wants to merge 1 commit into from

Conversation

rjzak
Copy link
Member

@rjzak rjzak commented Dec 9, 2022

Addresses #134, but doesn't close it, since this just reorganizes the test, but doesn't change them.

Inspired by Tokio's unit tests tokio-rs/tokio/tests.

Signed-off-by: Richard Zak [email protected]

@rjzak rjzak requested review from a team and bstrie as code owners December 9, 2022 17:25
@rjzak rjzak force-pushed the refactor_unit_tests branch from 3969e32 to d92af4c Compare December 9, 2022 17:34
@rjzak
Copy link
Member Author

rjzak commented Dec 9, 2022

After this, I'll:

  • Create a test which does the attestation of the signed CSRs based on attestation config
  • Remove old CSRs, so non-debug mode can be tested

@rjzak rjzak requested a review from rvolosatovs December 9, 2022 17:58
Copy link
Member

@rvolosatovs rvolosatovs left a comment

Choose a reason for hiding this comment

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

If we're still working on structure, can we just move all tests into one big tests/integration.rs? (and then we can either drop the isolated unit tests or move them to where they belong)

src/kvm.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
tests/common/validation.rs Outdated Show resolved Hide resolved
tests/common/config.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
tests/invalid.rs Outdated Show resolved Hide resolved
tests/encoding.rs Outdated Show resolved Hide resolved
tests/encoding.rs Outdated Show resolved Hide resolved
@rjzak rjzak force-pushed the refactor_unit_tests branch 2 times, most recently from 40bf5b1 to 7d5d6ea Compare December 9, 2022 19:40
@rjzak rjzak requested a review from haraldh as a code owner December 9, 2022 19:40
@rjzak rjzak marked this pull request as draft December 9, 2022 19:43
@rjzak rjzak force-pushed the refactor_unit_tests branch 3 times, most recently from 2ce33b7 to cb284b4 Compare December 9, 2022 20:48
@rjzak rjzak marked this pull request as ready for review December 9, 2022 22:20
@rjzak rjzak requested a review from rvolosatovs December 9, 2022 22:54
crates/server/Cargo.toml Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@rjzak rjzak force-pushed the refactor_unit_tests branch 3 times, most recently from 00812b7 to 89afbbc Compare December 12, 2022 15:34
@rjzak rjzak requested a review from haraldh December 12, 2022 15:46
@rjzak rjzak enabled auto-merge (rebase) December 12, 2022 15:46
@rjzak rjzak force-pushed the refactor_unit_tests branch from 89afbbc to 83c70fb Compare December 12, 2022 19:35
Copy link
Member

@rvolosatovs rvolosatovs left a comment

Choose a reason for hiding this comment

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

I don't think the commit message/PR title matches what this PR is now doing.
This is simply extracting the server implementation into its own crate. Please change the PR title and commit message to reflect that.

@rjzak rjzak changed the title chore: refactor unit tests chore: refactor server into server crate Dec 13, 2022
@rjzak rjzak force-pushed the refactor_unit_tests branch from 83c70fb to df008c8 Compare December 13, 2022 14:51
@rjzak
Copy link
Member Author

rjzak commented Dec 13, 2022

@rvolosatovs commit message and title reworded

@rjzak rjzak closed this Dec 13, 2022
auto-merge was automatically disabled December 13, 2022 15:54

Pull request was closed

@rjzak rjzak reopened this Dec 13, 2022
@rjzak rjzak force-pushed the refactor_unit_tests branch from df008c8 to f0a013f Compare December 13, 2022 16:12
@rvolosatovs rvolosatovs self-requested a review December 13, 2022 16:15
@rvolosatovs rvolosatovs reopened this Dec 13, 2022
@rvolosatovs rvolosatovs enabled auto-merge (rebase) December 13, 2022 16:20
@rvolosatovs rvolosatovs disabled auto-merge December 13, 2022 16:44
@rvolosatovs
Copy link
Member

Merged in 31ecb62
with the following diff:

diff --git a/Cargo.toml b/Cargo.toml
index 062699c..ce07a86 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,12 +10,12 @@ license = "AGPL-3.0"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-attestation = { path = "crates/attestation", features = ["sgx", "snp"] }
-steward-server = { path = "crates/server" }
 anyhow = { version = "^1.0.66", default-features = false }
+attestation = { path = "crates/attestation", features = ["sgx", "snp"] }
 axum = { version = "^0.5.17", features = ["headers"], default-features = false }
 clap = { version = "^4.0.29", features = ["help", "usage", "error-context", "std", "derive", "env"], default-features = false }
 confargs = { version = "^0.1.3", default-features = false }
+steward-server = { path = "crates/server" }
 tokio = { version = "^1.23.0", features = ["rt", "macros"], default-features = false }
 tower-http = { version = "^0.3.5", features = ["trace"], default-features = false }
 tracing = { version = "^0.1.29", default-features = false }
diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml
index 143c083..b892af0 100644
--- a/crates/server/Cargo.toml
+++ b/crates/server/Cargo.toml
@@ -6,8 +6,8 @@ license = "AGPL-3.0"
 description = "Server library for Steward"
 
 [dependencies]
-attestation = { path = "../../crates/attestation", features = ["sgx", "snp"] }
 anyhow = { version = "^1.0.66", default-features = false }
+attestation = { path = "../../crates/attestation", features = ["sgx", "snp"] }
 axum = { version = "^0.5.17", features = ["headers"], default-features = false }
 const-oid = { version = "0.9.1", features = ["db"], default-features = false }
 der = { version = "0.6", features = ["std"], default-features = false }
@@ -30,5 +30,5 @@ http = { version = "^0.2.6", default-features = false }
 memoffset = { version = "0.7.1", default-features = false }
 rstest = { version = "0.16", default-features = false }
 sgx = { version = "0.6.0", default-features = false }
-tower = { version = "^0.4.11", features = ["util"], default-features = false }
 testaso = { version = "0.1", default-features = false }
+tower = { version = "^0.4.11", features = ["util"], default-features = false }

@rjzak rjzak deleted the refactor_unit_tests branch December 13, 2022 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants