Ardia test stability and .gitignore fix (#3261)

* changed build-nt-x86 rule in .gitignore to work at any path level * disabled problematic section of Ardia functional test (currently test is 100% broken though due to apparent server-side change, still working on it) * disabled Mac GitHub Action build * added -q to CleanBiblioSpec.bat's git clean calls * moved TestConnected testing in Skyline Jamfile.jam from TestFunctional target to its own TestConnected target * added TestOrbiPrmArdia test to TC skiplist * added post-build git status check to ensure no non-ignored files were created * added SkylineWithTestConnected target to vcs_trigger_and_paths_config.py - updated msconvert-help.txt * added Bruker manifests to .gitignore * made install_pwiz_vendor_api_bruker_stub explicit target - fixed option description for BlibFilter -b (closes #3263)
ProteoWizard · Dec 6, 2024 · 862f0af · 862f0af
1 parent 59839d2
commit 862f0af
Show file tree

Hide file tree

Showing 11 changed files with 108 additions and 22 deletions.
diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml
@@ -57,12 +57,12 @@ jobs:
 #            compiler: g++-9,
 #            toolset: 'toolset=gcc'
 #          }
-        - {
-            name: "macOS Latest Clang",
-            os:	macos-12,
-            compiler: 'latest clang',
-            toolset: 'toolset=darwin'
-          }
+#        - {
+#            name: "macOS Latest Clang",
+#            os:	macos-12,
+#            compiler: 'latest clang',
+#            toolset: 'toolset=darwin'
+#          }
 
     # Using the value from the matrix above we select the platform (operating system) to run on
     runs-on: ${{ matrix.config.os }}

diff --git a/.gitignore b/.gitignore
@@ -360,7 +360,7 @@ ASALocalRun/
 # NVidia Nsight GPU debugger configuration file
 *.nvuser
 
-/build-nt-x86
+build-nt-x86
 /src_subset
 /*.log
 /libraries/boost_1_*
@@ -402,6 +402,7 @@ Version.cpp
 /pwiz_aux/msrc/utility/vendor_api/Bruker/x64/
 /pwiz_aux/msrc/utility/vendor_api/Bruker/x86/
 /pwiz_aux/msrc/utility/vendor_api/Bruker/**/*.h
+/pwiz_aux/msrc/utility/vendor_api/Bruker/**.SxS.manifest
 /pwiz_aux/msrc/utility/vendor_api/Mobilion/*.h
 /pwiz_aux/msrc/utility/vendor_api/Shimadzu/*.xml
 /pwiz_aux/msrc/utility/vendor_api/Shimadzu/*.txt

diff --git a/pwiz_aux/msrc/utility/vendor_api/Bruker/Jamfile.jam b/pwiz_aux/msrc/utility/vendor_api/Bruker/Jamfile.jam
@@ -233,4 +233,6 @@ explicit install_pwiz_vendor_api_bruker ;
 install install_pwiz_vendor_api_bruker_stub
     : [ path.glob $(API_PATH) : *.manifest ]
     ;
+
+explicit install_pwiz_vendor_api_bruker_stub ;
 }
diff --git a/pwiz_tools/BiblioSpec/CleanBiblioSpec.bat b/pwiz_tools/BiblioSpec/CleanBiblioSpec.bat
@@ -7,8 +7,8 @@ set PWIZ_ROOT=%~dp0
 set PWIZ_ROOT=%PWIZ_ROOT:~0,-1%
 pushd %PWIZ_ROOT%
 
-git clean -f -x tests\output
-git clean -f -x tests\inputs
+git clean -fq -x tests\output
+git clean -fq -x tests\inputs
 
 
 popd
diff --git a/pwiz_tools/BiblioSpec/src/BlibFilter.cpp b/pwiz_tools/BiblioSpec/src/BlibFilter.cpp
@@ -170,7 +170,7 @@ void BlibFilter::parseCommandLine(const int argc,
 
             ("best-scoring,b",
              value<bool>()->default_value(false),
-             "Description of option.  Default false.")
+             "Select only the spectrum with the best score for each peptide (spectrum TIC is used as tiebreaker).  Default false.")
 
             ;
 

diff --git a/pwiz_tools/Skyline/Jamfile.jam b/pwiz_tools/Skyline/Jamfile.jam
@@ -273,15 +273,34 @@ if [ modules.peek : NT ] && --i-agree-to-the-vendor-licenses in [ modules.peek :
         REM docker > nul 2>&1
         REM if ERRORLEVEL 1 (
             echo Functional testing Skyline in $(CONFIGURATION:L) configuration...
-            $(OUTPUT_PATH)TestRunner.exe log=TestFunctional.log buildcheck=1 test=TestFunctional.dll,TestConnected.dll $(.teamcity-test-decoration)
+            $(OUTPUT_PATH)TestRunner.exe log=TestFunctional.log buildcheck=1 test=TestFunctional.dll $(.teamcity-test-decoration)
             set status=%ERRORLEVEL%
         REM ) ELSE (
         REM    echo Functional testing Skyline in $(CONFIGURATION:L) configuration with $(WORKERS) Docker workers...
         REM     FOR /L %I IN (1,1,10) DO $(OUTPUT_PATH)TestRunner.exe log=TestFunctional.log parallelmode=server workercount=$(WORKERS) keepworkerlogs=1 waitforworkers=1 buildcheck=1 language=en loop=1 test=TestFunctional.dll,TestConnected.dll $(.teamcity-test-decoration)
         REM )
         exit %status%
     }
-
+
+    rule do_skyline_test_connected ( targets + : sources * : properties * )
+    {
+        return [ build-properties $(targets) : $(sources) : $(properties) ] ;
+    }
+
+    actions do_skyline_test_connected
+    {
+        $(MSVC_CURRENT_SETUP_SCRIPT)
+        chcp 437
+        echo Building Skyline TestConnected in $(CONFIGURATION:L) configuration...
+        msbuild $(SKYLINE_PATH)/Skyline.sln /p:Configuration=$(CONFIGURATION);Platform=$(PLATFORM);OutDir=$(OUTPUT_PATH);MSBuildAssemblyName=$(TARGET_NAME) /t:TestConnected /nologo /verbosity:minimal
+        IF ERRORLEVEL 1 exit %ERRORLEVEL%
+
+        echo Running Skyline's remote connection tests (TestConnected) in $(CONFIGURATION:L) configuration...
+        $(OUTPUT_PATH)TestRunner.exe log=TestConnected.log buildcheck=1 test=TestConnected.dll $(.teamcity-test-decoration)
+        set status=%ERRORLEVEL%
+        exit %status%
+    }
+
     rule do_skyline_test_passed ( targets + : sources * : properties * )
     {
         return [ build-properties $(targets) : $(sources) : $(properties) ] ;
@@ -550,7 +569,19 @@ if [ modules.peek : NT ] && --i-agree-to-the-vendor-licenses in [ modules.peek :
             <conditional>@build-location
             <dependency>Skyline.exe
         ;
-
+
+    make TestConnected
+        : # sources
+        : # actions
+            @do_skyline_test_connected
+        : # requirements
+            <link>shared:<build>no
+            <conditional>@no-express-requirement
+            <conditional>@msvc-dotnet-requirement
+            <conditional>@build-location
+            <dependency>Skyline.exe
+        ;
+
     make Skyline.passed
         : # sources
         : # actions

diff --git a/pwiz_tools/Skyline/TestConnected/ArdiaFunctionalTest.cs b/pwiz_tools/Skyline/TestConnected/ArdiaFunctionalTest.cs
@@ -215,7 +215,7 @@ protected override void DoTest()
             RemoveResultsAndReimport();
 
             // corrupt the cookie (simulate it being expired) and try reimporting again
-            ArdiaAccount.SetSessionCookieString(_account, "foobar" );
+            /*ArdiaAccount.SetSessionCookieString(_account, "foobar" );
             _account.ResetAuthenticatedHttpClientFactory();
 
             // delete local files
@@ -224,7 +224,7 @@ protected override void DoTest()
 
             Settings.Default.RemoteAccountList.Clear();
             Settings.Default.RemoteAccountList.Add(_account);
-            RemoveResultsAndReimport();
+            RemoveResultsAndReimport();*/
 
             ArdiaAccount.ClearSessionCookieStrings();
         }

diff --git a/pwiz_tools/commandline/msconvert-help.txt b/pwiz_tools/commandline/msconvert-help.txt
@@ -137,8 +137,9 @@ This filter adds or replaces spectrum titles according to specified <format_stri
    "<Id>" - prints the spectrum's nativeID
    "<SourcePath>" - prints the path of the spectrum's source data
    "<ScanNumber>" - if the nativeID can be represented as a single number, prints that number, else index+1
+   "<IonMobility>" - for the first scan of a spectrum, prints the "ion mobility drift time" or "inverse reduced ion mobility" value
    "<ActivationType>" - for the first precursor, prints the spectrum's "dissociation method" value
-   "<IsolationMz>" - for the first precursor, prints the the spectrum's "isolation target m/z" value
+   "<IsolationMz>" - for the first precursor, prints the spectrum's "isolation target m/z" value
    "<PrecursorSpectrumId>" - prints the nativeID of the spectrum of the first precursor
    "<SelectedIonMz>" - prints the m/z value of the first selected ion of the first precursor
    "<ChargeState>" - prints the charge state for the first selected ion of the first precursor
@@ -175,6 +176,14 @@ Filters spectra based on precursor m/z values found in the <precursor_mz_list>,
    <target> is optional and must be either "selected" (the default) or "isolated". It determines whether the isolated m/z or the selected m/z is used for the "precursor m/z"
    <mode> is optional and must be either "include" (the default) or "exclude".  If "exclude" is used, the filter drops spectra that match the various criteria instead of keeping them.
 
+isolationWindows <isolation_window_list> [mzTol=<mzTol (10 ppm)>] [mode=<include|exclude (include)>]
+Filters spectra based on precursor isolation windows found in the <isolation_window_list>, with <mzTol> m/z tolerance. To retain only spectra with isolation windows [123.4,153.4] and [567.8,617.8], use --filter "isolationWindows [123.4,153.4] [567.8,617.8]". Note that this filter will drop MS1 scans unless you include [0,0] in the list of isolation windows.   <mzTol> is optional and must be specified as a number and units (PPM or MZ). For example, "5 PPM" or "2.1 MZ".
+   <mode> is optional and must be either "include" (the default) or "exclude".  If "exclude" is used, the filter drops spectra that match the various criteria instead of keeping them.
+
+isolationWidth <isolation_width_list> [mzTol=<mzTol (10 ppm)>] [mode=<include|exclude (include)>]
+Filters spectra based on precursor isolation width values found in the <isolation_width_list>, with <mzTol> m/z tolerance. To retain only spectra with isolation width values of 1.5 and 2.3, use --filter "isolationWidth [1.5,2.3]". Note that this filter will drop MS1 scans unless you include 0.0 in the list of isolation width values.   <mzTol> is optional and must be specified as a number and units (PPM or MZ). For example, "5 PPM" or "2.1 MZ".
+   <mode> is optional and must be either "include" (the default) or "exclude".  If "exclude" is used, the filter drops spectra that match the various criteria instead of keeping them.
+
 defaultArrayLength <peak_count_range>
 Keeps only spectra with peak counts within <peak_count_range>, expressed as an int_set. (In mzML the peak list length is expressed as "defaultArrayLength", hence the name.)  For example, to include only spectra with 100 or more peaks, you would use filter "defaultArrayLength 100-" .
 
@@ -264,9 +273,10 @@ Predicts MSn spectrum precursor charge based on the isotopic distribution associ
   <half-width of isolation window> (default: 1.25): half-width of the isolation window (in Th.) from which precursor is derived. Window is centered at target m/z with a total size of +/- the value entered.
   <defaultMinCharge> (default: 0) and <defaultMaxCharge> (default: 0): in the event that no isotope is found in the isolation window, a range of charges between these two values will be assigned to the spectrum. If both values are left at zero, no charge will be assigned to the spectrum.
 
-activation <precursor_activation_type>
-Keeps only spectra whose precursors have the specifed activation type.  It doesn't affect non-MS spectra, and doesn't affect MS1 spectra. Use it to create output files containing only ETD or CID MSn data where both activation modes have been interleaved within a given input vendor data file (eg: Thermo's Decision Tree acquisition mode).
+activation <precursor_activation_type> [mode=<include|exclude (include)>]
+Keeps only spectra whose precursors have the specified activation type. It doesn't affect non-MS spectra, and doesn't affect MS1 spectra. Use it to create output files containing only ETD or CID MSn data where both activation modes have been interleaved within a given input vendor data file (eg: Thermo's Decision Tree acquisition mode).
    <precursor_activation_type> is any one of: ETD CID SA HCD HECID BIRD ECD IRMPD PD PSD PQD SID or SORI.
+   <mode> is optional and must be either "include" (the default) or "exclude". If "exclude" is used, the filter drops spectra that match the activation type instead of keeping them.
 
 collisionEnergy low=<real> high=<real> [mode=<include|exclude (include)>] [acceptNonCID=<true|false (true)] [acceptMissingCE=<true|false (false)]
 Includes/excludes MSn spectra with CID collision energy within the specified range [<low>, <high>].

diff --git a/scripts/misc/tc-perftests-skiplist.txt b/scripts/misc/tc-perftests-skiplist.txt
@@ -24,6 +24,7 @@ TestEncyclopeDiaSearchTutorialFullFileset
 TestFeatureDetectionTutorialFuture
 #TestImportHundredsOfReplicates
 #TestMobilIon
+TestOrbiPrmArdia
 TestPeakPerf
 TestPerfTicArea
 zzzNativeVsMz5_AbDiaChromatogramPerformanceTest

diff --git a/scripts/misc/tcbuild.bat b/scripts/misc/tcbuild.bat
@@ -25,19 +25,38 @@ if %CLEAN%==1 (
   if %EXIT% NEQ 0 set ERROR_TEXT=Error performing clean & goto error
 
   REM # check clean did not dirty repo (but postpone error until after quickbuild)
-  git status --porcelain | findstr . && set CLEAN_EXIT=1
-  if %CLEAN_EXIT% NEQ 0 echo Repository is dirty after clean script >&2
+  git ls-files --deleted | findstr . && set CLEAN_EXIT=1
+  if %CLEAN_EXIT% NEQ 0 (
+    echo Clean script deleted some files it should not have. 1>&2
+  )
 )
 
 REM # the -p1 argument overrides bjam's default behavior of merging stderr into stdout
 REM # the --abbreviate-paths argument abbreviates paths like .../ftr1-value/ftr2-value/...
 
 REM # call quickbuild to build and run tests
 echo ##teamcity[progressMessage 'Running quickbuild...']
+echo quickbuild.bat -p1 --abbreviate-paths --teamcity-test-decoration --verbose-test %ALL_ARGS%
 call quickbuild.bat -p1 --abbreviate-paths --teamcity-test-decoration --verbose-test %ALL_ARGS%
 set EXIT=%ERRORLEVEL%
 if %EXIT% NEQ 0 set ERROR_TEXT=Error running quickbuild & goto error
-if %CLEAN_EXIT% NEQ 0 set EXIT=%CLEAN_EXIT% & set ERROR_TEXT=Repository was dirty after clean script & goto error
+
+if %CLEAN_EXIT% NEQ 0 (
+  set EXIT=%CLEAN_EXIT%
+  set ERROR_TEXT=Clean script deleted some files it should not have.
+  git ls-files --deleted
+  goto error
+)
+
+REM # check that build did not create any files that are not in .gitignore
+set BUILD_CLEAN_EXIT=0
+git status --porcelain | findstr . && set BUILD_CLEAN_EXIT=1
+if %BUILD_CLEAN_EXIT% NEQ 0 (
+  set EXIT=%BUILD_CLEAN_EXIT%
+  set ERROR_TEXT=The build created or deleted some files that are not in .gitignore.
+  goto error
+)
+
 
 REM # uncomment this to test that test failures and error output are handled properly
 REM call quickbuild.bat -p1 --teamcity-test-decoration pwiz/utility/misc//FailUnitTest pwiz/utility/misc//FailRunTest

diff --git a/scripts/misc/vcs_trigger_and_paths_config.py b/scripts/misc/vcs_trigger_and_paths_config.py
@@ -46,6 +46,22 @@
 #targets['Skyline'] = merge(targets['SkylineRelease'], targets['SkylineDebug'])
 targets['Skyline'] = targets['SkylineRelease']
 
+targets['SkylineWithTestConnected'] = \
+{
+    'master':
+    {
+        "ProteoWizard_SkylineMasterAndPRsTestConnectedTests": "Skyline master and PRs TestConnected tests" # depends on "bt209",
+        ,"ProteoWizard_WindowsX8664msvcProfessionalSkylineResharperChecks": "Skyline code inspection" # depends on "bt209",
+        ,"bt209": "Skyline master and PRs (Windows x86_64)"
+    },
+    'release':
+    {
+        "ProteoWizard_SkylineReleaseTestConnectedTests": "Skyline release TestConnected tests" # depends on "ProteoWizard_WindowsX8664SkylineReleaseBranchMsvcProfessional",
+        ,"ProteoWizard_SkylineReleaseBranchCodeInspection": "Skyline release code inspection" # depends on "ProteoWizard_WindowsX8664SkylineReleaseBranchMsvcProfessional",
+        ,"ProteoWizard_WindowsX8664SkylineReleaseBranchMsvcProfessional": "Skyline Release Branch x86_64"
+    }
+}
+
 targets['Container'] = \
 {
     'master':
@@ -70,7 +86,7 @@
 targets['Bumbershoot'] = merge(targets['BumbershootRelease'], targets['BumbershootLinux'])
 
 targets['Core'] = merge(targets['CoreWindows'], targets['CoreLinux'])
-targets['All'] = merge(targets['Core'], targets['Skyline'], targets['Bumbershoot'], targets['Container'])
+targets['All'] = merge(targets['Core'], targets['SkylineWithTestConnected'], targets['Bumbershoot'], targets['Container'])
 
 # Patterns are processed in order. If a path matches multiple patterns, only the first pattern will trigger. For example,
 # "pwiz_tools/Bumbershoot/Jamfile.jam" matches both "pwiz_tools/Bumbershoot/.*" and "pwiz_tools/.*", but will only trigger "Bumbershoot" targets
@@ -83,6 +99,12 @@
     ("scripts/.*", targets['All']),
     ("pwiz_tools/BiblioSpec/.*", merge(targets['Core'], targets['Skyline'], targets['Container'])),
     ("pwiz_tools/Bumbershoot/.*", targets['Bumbershoot']),
+    ("pwiz_tools/Skyline/Model/Results/RemoteApi/.*", merge(targets['SkylineWithTestConnected'], targets['Container'])),
+    ("pwiz_tools/Skyline/.*Ardia.*", merge(targets['SkylineWithTestConnected'], targets['Container'])),
+    ("pwiz_tools/Skyline/.*Koina.*", merge(targets['SkylineWithTestConnected'], targets['Container'])),
+    ("pwiz_tools/Skyline/.*Panorama.*", merge(targets['SkylineWithTestConnected'], targets['Container'])),
+    ("pwiz_tools/Skyline/.*Unifi.*", merge(targets['SkylineWithTestConnected'], targets['Container'])),
+    ("pwiz_tools/Skyline/.*DataSource.*", merge(targets['SkylineWithTestConnected'], targets['Container'])),
     ("pwiz_tools/Skyline/.*", merge(targets['Skyline'], targets['Container'])),
     ("pwiz_tools/Shared/.*", merge(targets['Skyline'], targets['BumbershootRelease'], targets['Container'])),
     ("pwiz_tools/.*", targets['All']),