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

Improve test output regexes for better perf and Go 1.20 support #1220

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

refi64
Copy link
Contributor

@refi64 refi64 commented Sep 15, 2023

From the first commit:

The current regex runs in exponential time, which massively impacts the
runtime of the test suite, taking several seconds (~4s on my system)
just to perform a single match. By replacing the mix of re.findall + the
initial capture group with re.search + some string slicing, the time
spent matching the regex becomes nearly instant, e.g.:

    $ make system-test TESTS='Config*'

goes from taking ~10s to ~1.5s.

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

The current regex runs in exponential time, which massively impacts the
runtime of the test suite, taking several seconds (~4s on my system)
just to perform a single match. By replacing the mix of re.findall + the
initial capture group with re.search + some string slicing, the time
spent matching the regex becomes nearly instant, e.g.:

    $ make system-test TESTS='Config*'

goes from taking ~10s to ~1.5s.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
1.20 changes the output format of coverage checks slightly to include
a package name on each line, followed by `coverage:`, but the current
regex assumes that the line *starts* with `coverage:`.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #1220 (88f1409) into master (18203c6) will increase coverage by 0.01%.
Report is 5 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1220      +/-   ##
==========================================
+ Coverage   65.95%   65.96%   +0.01%     
==========================================
  Files         143      143              
  Lines       16192    16188       -4     
==========================================
  Hits        10679    10679              
+ Misses       4760     4756       -4     
  Partials      753      753              

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@reglim
Copy link
Contributor

reglim commented Sep 20, 2023

LGTM, @randombenj what do you think?

Also, would you mind adding your name to the AUTHOR file?

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
@refi64
Copy link
Contributor Author

refi64 commented Sep 22, 2023

@reglim added!

@reglim reglim merged commit 322e5c1 into aptly-dev:master Oct 3, 2023
4 checks passed
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.

None yet

2 participants