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

ci: Add system-tests #84

Merged
merged 5 commits into from
Jun 3, 2024
Merged

ci: Add system-tests #84

merged 5 commits into from
Jun 3, 2024

Conversation

cbeauchesne
Copy link
Contributor

@cbeauchesne cbeauchesne commented May 30, 2024

Add a job in circleCI that run system-tests. It will prevent introducing bug tested in system tests.

For now, I only added the default scenario, up to C++ team to decide what to add or remove later.

@cbeauchesne cbeauchesne force-pushed the cbeauchesne/system-tests branch from a0780d4 to af760d9 Compare May 30, 2024 14:30
@codecov-commenter
Copy link

codecov-commenter commented May 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.48%. Comparing base (f05587d) to head (43ab2c7).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #84   +/-   ##
=======================================
  Coverage   66.48%   66.48%           
=======================================
  Files          37       37           
  Lines        3554     3554           
  Branches      607      607           
=======================================
  Hits         2363     2363           
  Misses        875      875           
  Partials      316      316           

@cbeauchesne cbeauchesne force-pushed the cbeauchesne/system-tests branch 2 times, most recently from c2186aa to 6fa07b4 Compare May 30, 2024 15:11
@cbeauchesne cbeauchesne force-pushed the cbeauchesne/system-tests branch from 6fa07b4 to ca1a6f5 Compare May 30, 2024 16:21
@cbeauchesne cbeauchesne force-pushed the cbeauchesne/system-tests branch from 7ba143c to 38dbfca Compare May 30, 2024 17:44
@cbeauchesne cbeauchesne requested a review from Anilm3 May 31, 2024 12:32
@cbeauchesne cbeauchesne marked this pull request as ready for review May 31, 2024 12:33
@cbeauchesne cbeauchesne requested a review from a team as a code owner May 31, 2024 12:33
Comment on lines +661 to +664
- system_tests:
name: Run system tests
requires:
- build 1.25.4 on amd64 WAF ON
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 it's necessay for build-and-test but not for build-and-test-all ?

@dmehala dmehala changed the title [CI] Add system-tests ci: Add system-tests May 31, 2024
@dmehala
Copy link
Contributor

dmehala commented May 31, 2024

LGTM, thank you @cbeauchesne. However I can see in the system-tests output:

Library: [email protected]
Weblog variant: nginx

How the weblog versioning is working? Is there a weblog versioning?

@cbeauchesne
Copy link
Contributor Author

aha, good catch, I didn't expect you to see this. There are several issue here.

Currently, the version number for c++ is the version number of this present repo, where it should be the version of dd-trace-cpp

Second, I need to have a way to extract this version number from the ngx_http_datadog_module.so file, and I don't know how to do so. So shame on me, I went lazy, and I hardcoded a big version number, leaving this point for later.

If you know how to extract this version number, I can add it (it's inside system-tests repo). If you don't know, or if it's not possible (yet), my opinion is we can move forward with that hack, as long as the hardocded version is bigger than the current version, everything will works fine.

@cataphract
Copy link
Contributor

aha, good catch, I didn't expect you to see this. There are several issue here.

Currently, the version number for c++ is the version number of this present repo, where it should be the version of dd-trace-cpp

Second, I need to have a way to extract this version number from the ngx_http_datadog_module.so file, and I don't know how to do so. So shame on me, I went lazy, and I hardcoded a big version number, leaving this point for later.

They can be fetched like this.

See also: https://github.com/DataDog/nginx-datadog/blob/master/src/version.cpp.in

@cbeauchesne
Copy link
Contributor Author

cbeauchesne commented May 31, 2024

thanks @cataphract , let me add this

-> DataDog/system-tests#2517

@dmehala
Copy link
Contributor

dmehala commented May 31, 2024

They can be fetched like this.

That's smart. We should also those this for dd-trace-cpp instead of parsing version.h. This would have avoid the issue with prod we got this week

@cbeauchesne
Copy link
Contributor Author

@dmehala could you merge the Pr? I don't have the right to do it myself 🙏

@dmehala dmehala merged commit 5e2e306 into master Jun 3, 2024
33 checks passed
@dmehala dmehala deleted the cbeauchesne/system-tests branch June 3, 2024 10:01
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.

4 participants