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

semver.xq should be tested against the more recent releases of exist 4, 5 and 6 #25

Open
line-o opened this issue Jan 8, 2023 · 7 comments

Comments

@line-o
Copy link
Member

line-o commented Jan 8, 2023

At the moment this library is tested against

  • eXist-db 4.2.0
  • eXist-db 5.1.0
  • eXist-db 6.1.0-SNAPSHOT

These versions should be added

  • eXist-db 4.10.0
  • eXist-db 5.4.0 (could replace 5.1.0)
  • eXist-db 6.0.1
@line-o line-o changed the title semver.xq should be tested against the more recent releases of exist 4 and 5 semver.xq should be tested against the more recent releases of exist 4, 5 and 6 Jan 8, 2023
@joewiz
Copy link
Member

joewiz commented Jan 8, 2023

In #15 (comment) where the current testing array was set, I asked:

Could you explain the thinking behind targeting versions 4.2.0 and 5.1.0 - which are neither the oldest nor the newest releases in their respective major series?

@adamretter responded:

They are the most stable versions that I could execute the code against. I looked for the oldest possible versions to ensure the most compatibility possible

I like the idea of ensuring maximal compatibility - as evidenced by my approval and merger of that PR.

However, another principle for determining versions is to choose the latest version within each major release as a target for testing. This way, the tested release contains all bugfixes we have put into that major version. Users should, in principle, be able to update to the latest release in a major series without risking breakage. If we target the oldest possible version within a major series, then the project won't be able to count on the bugfixes that are already in later versions of the same major series. So on balance I would be in favor of adopting this principle - here and across eXist-db organization-maintained packages.

@adamretter
Copy link
Contributor

adamretter commented Jan 10, 2023

the project won't be able to count on the bugfixes that are already in later versions of the same major series.

I think my point is that with appropriate testing (as I added to CI) we have proven that this project doesn't need such later bug-fixes from eXist-db. If this project was to require a newer version of eXist-db (due to some desired fix or feature) then it could certainly advance the version number of eXist-db it depends on, but otherwise there is no point artificially depending on a newer version of eXist-db where it isn't needed.

Minimum version compatibility (where possible) ensures the largest possible audience and user-base for this project, which I think we can all agree is the desired outcome.

@duncdrum
Copy link
Contributor

Testing against a minimal version 4.2 is desirable. Testing against the minimal version which receives community support 4.10 is a requirement. As we agreed ages ago, that supporting all possible versions isn’t feasible in light of the communities limited time. So we pick the latest in a release cycle for support.

@adamretter
Copy link
Contributor

Testing is not the same as minimum compatibility! Feel free to add additional versions to CI config if you believe they are really warranted.

@duncdrum
Copy link
Contributor

#20

@joewiz
Copy link
Member

joewiz commented Jan 11, 2023

@adamretter I am curious, when I run mvn test in a local checkout of the repo, which version of eXist is actually being used to execute the tests?

I ask because I noticed a pattern during my recent development sprints, when my local instance would pass with mvn test, and CI would pass with eXist 4.2.0 - but fail with eXist 5 and 6. Puzzled, I'd then need to look at the CI logs to see what these versions tripped on, then fix, then force push my commits again. I began to mistrust the results of mvn test and instead to run my xqsuites manually on local instances of eXist 5.1.0 and eXist 6.1.0-SNAPSHOT.

...

Looking into the ci.yml, I realize now that perhaps I should be running these 3 commands each time I want to get a comprehensive test result:

# test with 4.2.0
mvn verify
# test with 5.1.0
mvn verify -Pexistdb-5-or-newer -Dexist.version=5.1.0
# test with 6.1.0-SNAPSHOT
mvn verify -Pexistdb-5-or-newer -Dexist.version=6.1.0-SNAPSHOT

That would be really great to be able to quickly run the tests like this locally on all of the "representative" versions of eXist, instead of relying on CI. I'll try this the next time I work on the repo.

But I am genuinely flummoxed why so many tests (incorrectly) passed under 4.2.0 that (correctly) failed under 5.1.0 and 6.1.0-SNAPSHOT. I wish I'd made note of which did, because I'd really like to see if 4.2.0 is a dependable target for this package. I'll keep an eye on this and report any discoveries.

@adamretter
Copy link
Contributor

@adamretter I am curious, when I run mvn test in a local checkout of the repo, which version of eXist is actually being used to execute the tests?

I would recommend running mvn verify rather than mvn test.
By default the tests are executed against eXist-db 4.2.0.

That would be really great to be able to quickly run the tests like this locally on all of the "representative" versions of eXist

Sure, just run:

mvn verify && mvn verify -Pexistdb-5-or-newer -Dexist.version=5.1.0 && mvn verify -Pexistdb-5-or-newer -Dexist.version=6.1.0-SNAPSHOT

I'll keep an eye on this and report any discoveries.

Okay. I would need to actually see the details of where you thing the false-positives are occuring.

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

No branches or pull requests

4 participants