Skip to content

Conversation

@snuyanzin
Copy link
Collaborator

@snuyanzin snuyanzin commented Apr 24, 2024

the idea is to get main repo from datafaker-gen and build it and then run tests against latest datafaker version to be able to detect issue appearing only in downstream projects like e.g. #1170 or #1168

currently this check is done only in case of jdk21 and ubuntu (in fact i do not see a real reason to test it with the whole matrix)

@what-the-diff
Copy link

what-the-diff bot commented Apr 25, 2024

PR Summary

  • Introduction of an 'outputs' section in 'job1'
    An 'outputs' section was added to the 'job1' in our workflow automation file named 'maven.yml'. Essentially, this means our system will now provide additional information or results when 'job1' is finished executing.

  • Addition of a new job called 'datafaker-gen-test'
    A new task, 'datafaker-gen-test', which runs on the 'ubuntu-latest' operating system, was added to our workflow in 'maven.yml'. The addition of this task expands the scope of automated functions we can perform without human involvement.

  • New steps added to 'datafaker-gen-test' job
    Several steps have been added to guide the execution of 'datafaker-gen-test'. This includes setting up Java Development Kit (JDK) 21, checking out a specific repository named 'datafaker-net/datafaker-gen', and performing a 'clean install'. Achieving this means our system is now more self-reliant, performing necessary tasks like system setup and installing required software resources.

  • Utilizing the output from 'job1' in 'datafaker-gen-test' job
    The output generated from 'job1' is now being used as an environment variable in the 'datafaker-gen-test' job. This enhancement provides better integration between our jobs, ensuring that the output of one job can be beneficially used in another.

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.46%. Comparing base (b37c566) to head (e8495a8).
Report is 81 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1171      +/-   ##
============================================
+ Coverage     92.35%   92.46%   +0.11%     
- Complexity     2821     2839      +18     
============================================
  Files           292      293       +1     
  Lines          5609     5615       +6     
  Branches        599      598       -1     
============================================
+ Hits           5180     5192      +12     
+ Misses          275      271       -4     
+ Partials        154      152       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

name: Jdk ${{ matrix.java-version }}, os ${{ matrix.runs-on }}
runs-on: ${{ matrix.runs-on }}
outputs:
output1: ${{ steps.getProjectVersion.outputs.test }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not actually call it version or something meaningful?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

still playing to get something working
after that will rename it

@snuyanzin snuyanzin changed the title test Add tests agaisnt downstream project (datafaker-gen) Apr 25, 2024
@snuyanzin snuyanzin marked this pull request as ready for review April 25, 2024 14:11
@snuyanzin snuyanzin requested a review from kingthorin April 25, 2024 14:11
@snuyanzin
Copy link
Collaborator Author

//cc @bodiam since you were asking about something like this

@snuyanzin snuyanzin changed the title Add tests agaisnt downstream project (datafaker-gen) Add tests against downstream project (datafaker-gen) Apr 25, 2024
cache: maven
- name: Build with Maven
run: ./mvnw $mvn_options verify -PnoGpg --file pom.xml
run: ./mvnw $mvn_options install -PnoGpg --file pom.xml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we need install since during tests with downstream project these artifacts will be reused

Copy link
Contributor

Choose a reason for hiding this comment

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

What would you think of turning all the tests of Datafaker into a test artifact, and run them as part of the downstream project?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess most of them are independent whether they are running within Datafaker code or some downstream projects.
There are only few of them where it works differently.

Also there will be another challenge: how to make it possible
I see several solutions and none of them is perfect

  1. copy-paste of tests however it means we need to maintain 2 repos synced at least from tests point of view
  2. we could publish test-jar and reuse it in downstream project however anyway need to create a bunch of child classes and keep it sync (e.g. each time a new test class is appearing in Datafaker we need to create a corresponding child in third party)
  3. something else that I don't know yet

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't understand point 2. Why would we need to create child classes? The idea I had was to indeed create a test artifact, put all the tests in there, and use surefire to run the tests against the jar. I'm not sure if we do some magic which prevents this?

Alternatively, I think we need only 1 test, which is very similar to the repeatability test : just call every in the public api and make sure it returns something. That would have stopped a few bugs from being released.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

May be I missed something however I thought that tests will not run if we just add a test-jar dependency without any usage of that

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, here it says we could run them if we package them up and give them to surefire, unless I'm misunderstanding something:

https://stackoverflow.com/questions/10496846/run-junit-tests-contained-in-dependency-jar-using-maven-surefire

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

may be makes sense to try it once more
not sure when will have time for this

@snuyanzin snuyanzin merged commit 872e3ab into datafaker-net:main Apr 25, 2024
@kingthorin
Copy link
Collaborator

Sorry I missed the request on this one. It does look good 🙂

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