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

test: Collect client test data from E2E tests (DSP-724) #1724

Merged
merged 38 commits into from
Oct 9, 2020

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Oct 2, 2020

resolves DSP-724

  • Add ClientTestDataCollector class that E2E tests can use.
  • Move client test data generation from routes and SharedTestDataADM into E2E tests:
    • API v2 routes:
      • ResourcesRouteV2 -> ResourcesRouteV2E2ESpec
      • OntologiesRouteV2 -> OntologyV2R2RSpec
      • ValuesRouteV2 -> ValuesRouteV2E2ESpec
      • SearchRouteV2 -> SearchRouteV2R2RSpec
      • ListsRouteV2 -> ListsRouteV2R2RSpec
    • Admin routes:
      • UsersRouteADM -> UsersADME2ESpec
      • GroupsRouteADM -> GroupsADME2ESpec
      • ProjectsRouteADM -> ProjectsADME2ESpec
      • PermissionsRouteADM -> PermissionsADME2ESpec
      • ListsRouteADM -> ListsADME2ESpec
  • Change Makefile to do the following when running E2E tests:
    • Start the Redis container defined in docker-compose.yml.
    • Remove any client test data previously stored in Redis.
    • Remove any previously downloaded Zip file containing client test data.
    • After tests complete, download the client test data from Redis into a Zip file.
    • Check the contents of the the downloaded Zip file.
  • Move shared test data classes from main to test.
  • Update docs.

@benjamingeer benjamingeer self-assigned this Oct 2, 2020
@benjamingeer benjamingeer added chore maintenance and build tasks API/Admin API/V2 labels Oct 2, 2020
@subotic
Copy link
Collaborator

subotic commented Oct 8, 2020

When you add a separate Makefile target for the generation of the client-test-data, maybe it would also make sense to add an additional Github-CI job for this target?

@benjamingeer
Copy link
Author

 When you add a separate Makefile target for the generation of the client-test-data

I thought of that, but the problem is as I wrote above: if Redis isn't running, the E2E tests will fail, because ClientTestDataCollector will throw an exception if it fails to connect to Redis. We would need some way of telling it not to try to connect to Redis if it isn't being run by that Makefile target.

@benjamingeer
Copy link
Author

I guess I could do it with an environment variable...

@subotic
Copy link
Collaborator

subotic commented Oct 8, 2020

You could add a KnoraSetting, which could be configured through an environment variable. The default could be false, i.e., don't try to actually write to the cache. You could then test for it in the TestDataCollector implementation.

@subotic
Copy link
Collaborator

subotic commented Oct 9, 2020

@benjamingeer if you need help, I could quickly add the environment variable part. Just let me now.

@subotic
Copy link
Collaborator

subotic commented Oct 9, 2020

@subotic
Copy link
Collaborator

subotic commented Oct 9, 2020

Regarding the health route, @tobiasschweizer would like that the data goes into a system folder and not admin or v2.

@benjamingeer
Copy link
Author

There is an e2e test, which responses should also be gathered

Let's do that in a separate PR.

@subotic
Copy link
Collaborator

subotic commented Oct 9, 2020

There is an e2e test, which responses should also be gathered

Let's do that in a separate PR.

Just wanted to suggest that. There is a new Youtrack story: https://dasch.myjetbrains.com/youtrack/issue/DSP-743

@benjamingeer
Copy link
Author

You could add a KnoraSetting, which could be configured through an environment variable. The default could be false, i.e., don't try to actually write to the cache. You could then test for it in the TestDataCollector implementation.

I implemented it in fd0d3cf, but it doesn't work: KnoraSettings never gets the value of the environment variable. Could you have a look?

@subotic
Copy link
Collaborator

subotic commented Oct 9, 2020

You could add a KnoraSetting, which could be configured through an environment variable. The default could be false, i.e., don't try to actually write to the cache. You could then test for it in the TestDataCollector implementation.

I implemented it in fd0d3cf, but it doesn't work: KnoraSettings never gets the value of the environment variable. Could you have a look?

sure, will do after lunch

@subotic
Copy link
Collaborator

subotic commented Oct 9, 2020

@benjamingeer the generation works now.

@benjamingeer
Copy link
Author

the generation works now.

Excellent, thank you! Could you give me a hint about how to do this:

To use the SharedTestDataADM object in it, you would need to add a dependency in the Bazel build file which points to the Bazel package under test.

@benjamingeer
Copy link
Author

To use the SharedTestDataADM object in it, you would need to add a dependency in the Bazel build file which points to the Bazel package under test.

OK I figured it out. I think that's everything you asked for.

@benjamingeer benjamingeer requested a review from subotic October 9, 2020 13:26
Copy link
Collaborator

@subotic subotic left a comment

Choose a reason for hiding this comment

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

Great, thanks for the work!

@benjamingeer
Copy link
Author

@subotic Thanks for the review and the help!

@SepidehAlassi SepidehAlassi merged commit 743cd7e into develop Oct 9, 2020
@SepidehAlassi SepidehAlassi deleted the wip/DSP-724-client-test-data branch October 9, 2020 14:39
To generate client test data, type:

```
make test-e2e
Copy link
Contributor

Choose a reason for hiding this comment

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

@benjamingeer I believe this had to be updated to make client-test-data, right?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, sorry, I forgot that.

When the tests have finished running, you will find the file
`client-test-data.zip` in the current directory.

If generated client test data changes, run `make test-e2e`, then run
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, this has to be make client-test-data, right? If so, I can update this documentation.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API/Admin API/V2 breaking any breaking change chore maintenance and build tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants