- 
                Notifications
    
You must be signed in to change notification settings  - Fork 72
 
Not --all-features test suite #1038
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
base: master
Are you sure you want to change the base?
Conversation
1158ab4    to
    c367b20      
    Compare
  
    
          Pull Request Test Coverage Report for Build 18915659489Details
 
 
 💛 - Coveralls | 
    
c367b20    to
    ee62384      
    Compare
  
            
          
                payjoin/contrib/test.sh
              
                Outdated
          
        
      | cargo test --locked --package payjoin --verbose --no-default-features --features v1 --test integration | ||
| 
               | 
          ||
| cargo test --locked --package payjoin --verbose --no-default-features --features v2 --lib | ||
| cargo test --locked --package payjoin --verbose --no-default-features --features v2 --test integration | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on the current cfg logic in the integration tests no tests run when v2 is the only thing enabled, perhaps someone could point me in the right direction on better splitting up the tests based on features
8e300b4    to
    273378f      
    Compare
  
    273378f    to
    a0e0e98      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess is that the v2 tests are relying on the URI builder from v1. We may need another builder that's v2-specific OR to expose this utility without feature gating it. The former seems more appropriate.
let mut pj_uri =
                build_v1_pj_uri(&pj_receiver_address, EXAMPLE_URL, OutputSubstitution::Enabled)?;4439740    to
    cea604e      
    Compare
  
    
          
 Its only the v2_to_v1 test that requires this setup whereas all the others rely on the usual session to create a pj_uri. I guess we could create a pj_uri with a session like that but then it really wouldn't be a v1 receiver  | 
    
| 
           Ah, and in that case it makes sense to test only with v1 compiled. Needs further investigation I suppose.  | 
    
a258176    to
    ace4cf6      
    Compare
  
    257cc45    to
    b1f803f      
    Compare
  
    32abaa0    to
    91db3c6      
    Compare
  
    91db3c6    to
    b790f30      
    Compare
  
    The uri tests were looking at bip21 uris with v1 or v2 specific features and failing when the features were locked to the opposite feature respectively. I have moved the relevant tests to be in the correct module so that they only compile within the correct feature.
This commit locks the integration tests down into their specific enabled versions.
Add the test suite for the `contrib/test,sh` script in the payjoin crate.
b790f30    to
    65f2cfd      
    Compare
  
    
Got everything compiling, however based on the current cfg flag layout in the integration tests, all the tests are run when v1 is enabled and none of the tests run when v2 is enabled
Closes #1019
Pull Request Checklist
Please confirm the following before requesting review:
I have disclosed my use of
AI
in the body of this PR.
I had chatgpt help me clean up some clones in the
SenderBuilderwhile i was transitioning over to theNoopSessionPersisterI have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.