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

[C4GT] Network-Score: Add tests for 'MessageIdAlreadyExists' #296

Closed
3 tasks
vatsa287 opened this issue Feb 5, 2024 · 7 comments · Fixed by #330
Closed
3 tasks

[C4GT] Network-Score: Add tests for 'MessageIdAlreadyExists' #296

vatsa287 opened this issue Feb 5, 2024 · 7 comments · Fixed by #330
Assignees
Labels

Comments

@vatsa287
Copy link
Member

vatsa287 commented Feb 5, 2024

Description

Subtask under : cord-network/community#7

New testcase can be tested under cargo test -p pallet-network-score after adding it in the code.

Goals

  • Add tests for MessageIdAlreadyExists for pallet/network-score

Expected Outcome

  • Test should assert for MessageIdAlreadyExists being returned properly in all the possible calls.
  • If there are more than one function returning this error code, all of them should be validated either as one test case or as multiple test cases.

Acceptance Criteria

NA

Implementation Details

Look at other test cases, and add a test case for the same.
Ex: In pallets/network-membership check method test_duplicate_member_request which validates error code MembershipAlreadyAcquired

Mockups / Wireframes

NA


Product Name

CORD

Organization Name

Dhiway

Domain

Blockchain

Tech Skills Needed

Rust

Mentor(s)

@amarts

Complexity

[Low]

Category

[Test]

Sub Category

[Beginner friendly]

@amarts amarts added the good first issue Good for newcomers label Feb 6, 2024
@zeel991
Copy link
Contributor

zeel991 commented Feb 7, 2024

Hey @amarts , In this project we have to make a test which asserts that message already exists and we need to add this test in to network-score folder and that should pass three tests , namely Build , Cargo and test mentioned in the Readme.md File , Have I got the right idea ? let me know if I am wrong otherwise Tell me so i start to working on it
Thank you

@vatsa287
Copy link
Member Author

vatsa287 commented Feb 8, 2024

@zeel991 Yes you have to add new test method in tests.rs which,
asserts the error code MessageIdAlreadyExists for all methods used in lib.rs.
Example: MessageIdAlreadyExists is being returned from methods revise_rating, register_rating etc.
Then call cargo test -p pallet-network-score. If it passes proceed with pre PR checks as mentioned in README.

@zeel991
Copy link
Contributor

zeel991 commented Feb 8, 2024

Ok , I'll start working on this issue as the next thing .Thank you

@zeel991
Copy link
Contributor

zeel991 commented Feb 10, 2024

Hey @vatsa287 ,I have been trying to build cord in production mode using the command cargo build --production , but it is not working and giving error , It shows that unexpected argument '--production' found , my cargo version is 1.75.0 , I tried the command cargo build --release , which worked fine , what should I do , can you help solve the error ?
Screenshot 2024-02-10 160615

@vatsa287
Copy link
Member Author

vatsa287 commented Feb 11, 2024

@zeel991 Yes you can proceed to with cargo build --release.

@amarts Maybe I'll update README to reflect change from --production to --release.

@zeel991
Copy link
Contributor

zeel991 commented Feb 11, 2024

Thank you so much @vatsa287 , I wanted to share that while running the clippy command ( cargo clippy --all --no-deps --all-targets --features=runtime-benchmarks -- -D warnings) , I found a few errors , one was
Screenshot 2024-02-12 010954
I've currently just commented it out , but would appreciate your advice on this ,

After commenting , I got this Error and tried very hard but couldn't fix it
Screenshot 2024-02-12 012544
Can you Please Guide me as I have written the test but these issues wont let me get my PR merged .
Thank you

@vatsa287
Copy link
Member Author

@zeel991 Interesting open the PR, lets check with GH tests.

This was referenced Feb 12, 2024
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 a pull request may close this issue.

3 participants