Skip to content

Conversation

jayantk
Copy link
Contributor

@jayantk jayantk commented Apr 13, 2023

Final step in fixing this repo. You can now simply run cargo test and it will compile the code (including the BPF binary) and run all of the oracle program tests. I also updated the README instructions, so please check them out and see if they work for you.

There is technically still one oracle program test in the pctest folder, but that test is not very good so I don't think it makes sense to move over. It only affects the twap calculation, which we already have decent test coverage of. I may add some more TWAP tests in the future so that we can update to the new version of the calculation.

main( int argc,
char ** argv ) {
(void)argc; (void)argv;
int test_price_model() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the changes in the C test files are:

  1. export a function name so we can call them from rust (instead of main)
  2. delete the noisy printf statements
  3. I reduced the iteration counts on a couple of the randomized tests, because they took several minutes to run.

@@ -1,91 +0,0 @@
#include <stdio.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file is like a super exhaustive test of the prng (which itself is only used for tests). It's not actually run as part of the existing c test suite, so just get rid of it.

@@ -63,8 +63,6 @@ RUN ./pyth-client/scripts/patch-solana.sh
# Build and test the oracle program.
RUN cd pyth-client && ./scripts/build-bpf.sh .
RUN cd pyth-client && ./scripts/check-size.sh
# Run aggregation logic tests
RUN cd pyth-client && ./scripts/run-aggregation-tests.sh
Copy link
Contributor Author

@jayantk jayantk Apr 13, 2023

Choose a reason for hiding this comment

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

Note that build-bpf.sh above runs cargo test-bpf, which now runs all of these tests.

@@ -73,9 +69,6 @@ main( int argc,
if( z[i]<=z[i-1] ) { printf( "FAIL (%s)\n", BEFORE( z[i], z[i-1] ) ? "order" : "stable" ); return 1; }
}

prng_delete( prng_leave( prng ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to delete this prng_delete( prng_leave( prng ) );

prng_t _prng[1];
prng_t * prng = prng_join( prng_new( _prng, (uint32_t)0, (uint64_t)0 ) );

int ctr;

ctr = 0;
for( int i=0; i<1000000000; i++ ) {
if( !ctr ) { printf( "reg: Completed %i iterations\n", i ); ctr = 10000000; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can delete ctr from every test since it was just used for printing

@@ -76,8 +73,8 @@ main( int argc,
# define N 512

ctr = 0;
for( int i=0; i<10000000; i++ ) {
Copy link
Contributor

@guibescos guibescos Apr 13, 2023

Choose a reason for hiding this comment

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

There's another variable called i inside the loop, can you fix that for clarity? For example by renaming this i to iter

guibescos
guibescos previously approved these changes Apr 13, 2023
Copy link
Contributor

@guibescos guibescos left a comment

Choose a reason for hiding this comment

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

Please check out the minor comments

@jayantk jayantk merged commit 71d2394 into main Apr 13, 2023
@jayantk jayantk deleted the fix_tests3 branch April 13, 2023 19:33
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