Skip to content

Conversation

@happyhappy-jun
Copy link
Contributor

@happyhappy-jun happyhappy-jun commented Jul 9, 2022

Type of this PR

Select at least one type

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Bug fix
  • Dependency, Environment, Build related update
  • Documentation
  • Other tiny fix

Describe your changes

  • 컨트랙트 인터랙션을 제외한 니어 노드 인터랙션을 구현했습니다.
  • reqwest 를 사용해 json rpc 형태로 리퀘스트를 노드로 날립니다.
  • json 리스폰스는 serde_json 으로 파싱합니다.

Issue ticket number and other helpful resource

#1

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • Leave reference about this pull request, if exist (ex. discord message, google docs, etc)
  • If it is a core feature, I have added thorough tests.

Checklist after creating a pull request

  • Mention reviews in discord channel directly

@happyhappy-jun happyhappy-jun requested a review from junha1 July 9, 2022 14:27
Copy link
Member

@junha1 junha1 left a comment

Choose a reason for hiding this comment

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

CI에도 config 전달을 해줘야 할 것 같네요.

tokio = { version = "1.0", features = ["full"] }
futures = "0.3"
reqwest = "0.11"
reqwest = { version = "0.11", features = ["json"] }
Copy link
Member

Choose a reason for hiding this comment

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

무관한 커밋에 들어있습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 3497363


let first_json = first_res.json::<Value>().await.unwrap();

thread::sleep(time::Duration::from_secs(2));
Copy link
Member

Choose a reason for hiding this comment

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

async 함수에서는 https://docs.rs/tokio/latest/tokio/time/fn.sleep.html 를 씁시다.

#[tokio::test]
#[ignore]
async fn check_account() {
let _config = Config::read_from_env();
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

#[tokio::test]
#[ignore]
async fn check_connection() {
let _config = Config::read_from_env();
Copy link
Member

Choose a reason for hiding this comment

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

변수명 앞에 _를 붙이는건 이 변수가 당장 안쓰인다는 것을 인지하고 있다는걸 알려주는 특수 린팅 규칙입니다. 이제 사용하니까 떼면 돼요.

#[tokio::test]
#[ignore]
async fn check_block_number() {
let _config = Config::read_from_env();
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

.unwrap();

let json = res.json::<Value>().await.unwrap();
assert_eq!(json["error"], Value::Null);
Copy link
Member

Choose a reason for hiding this comment

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

has enough native token to pay gas fee 에 대한 테스트 내용이 없는 것 같네요.

@happyhappy-jun happyhappy-jun force-pushed the feat-implement-simple-interact branch from ad68263 to 3497363 Compare July 9, 2022 17:35
@happyhappy-jun
Copy link
Contributor Author

CI config 전달에 대해 좋은 아이디어 받습니다

  1. .env 처럼 공용 env 파일을 만든다
  2. github action secret 에 env 로 넣는다
  3. github action yaml 에 env 로 넣는다

현재의 목적이 각 테스트마다 다른 환경변수를 넣어준다거나 그런것이 목적이라면, 1,2,3 영 아닌거 같은데 좋은 의견 있으신가요

@junha1
Copy link
Member

junha1 commented Jul 9, 2022

@happyhappy-jun 그냥 CI에서 사용할 env파일을 지금처럼 repo 어딘가에 두고 그대로 쓰는게 제일 간단할 것 같습니다. 만약에 컨트랙트를 테스트넷에 재배포한다거나 하면 주소가 바뀔테니 적절히 업데이트 해주고요.

@happyhappy-jun happyhappy-jun force-pushed the feat-implement-simple-interact branch from 7daaa74 to fa4e428 Compare July 17, 2022 01:19
@happyhappy-jun happyhappy-jun force-pushed the feat-implement-simple-interact branch from cc42fca to fab612f Compare July 20, 2022 13:54
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.

2 participants