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

Add (very few) unit tests to galley. #1071

Merged
merged 5 commits into from
Apr 23, 2020
Merged

Add (very few) unit tests to galley. #1071

merged 5 commits into from
Apr 23, 2020

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Apr 22, 2020

Some tests for the broadcast query parameters, since I was asked the question how they work and couldn't answer. Now the haddocks tells us, and if it's lying the CI won't pass. :)

One might argue that unit tests (tasty, probably) are more consistent with the rest of our tests. I think it's fine to have both, but if we agree on that, I am fine with moving the tests here to a unit test suite for galley (also to be created).

@fisx
Copy link
Contributor Author

fisx commented Apr 22, 2020

cd services galley && stack test --fast . works locally, but ci says:

[...]
> src/Galley/Aws.hs:35:1: error:
>     Could not load module ‘Blaze.ByteString.Builder’
>     It is a member of the hidden package ‘blaze-builder-0.4.1.0’.
>     You can run ‘:set -package blaze-builder’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:38:1: error:
>     Could not load module ‘Control.Monad.Trans.AWS’
>     It is a member of the hidden package ‘amazonka-1.6.1’.
>     You can run ‘:set -package amazonka’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:39:1: error:
>     Could not load module ‘Control.Monad.Trans.Resource’
>     It is a member of the hidden package ‘resourcet-1.2.2’.
>     You can run ‘:set -package resourcet’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:42:1: error:
>     Could not load module ‘Data.ProtoLens.Encoding’
>     It is a member of the hidden package ‘proto-lens-0.5.1.0’.
>     You can run ‘:set -package proto-lens’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:48:1: error:
>     Could not load module ‘Network.AWS’
>     It is a member of the hidden package ‘amazonka-1.6.1’.
>     You can run ‘:set -package amazonka’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:49:1: error:
>     Could not load module ‘Network.AWS.Env’
>     It is a member of the hidden package ‘amazonka-1.6.1’.
>     You can run ‘:set -package amazonka’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:50:1: error:
>     Could not load module ‘Network.AWS.SQS’
>     It is a member of the hidden package ‘amazonka-sqs-1.6.1’.
>     You can run ‘:set -package amazonka-sqs’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:56:1: error:
>     Could not load module ‘Network.TLS’
>     It is a member of the hidden package ‘tls-1.4.1’.
>     You can run ‘:set -package tls’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> 
> src/Galley/Aws.hs:57:1: error:
>     Could not load module ‘Proto.TeamEvents’
>     It is a member of the hidden package ‘types-common-journal-0.1.0’.
>     You can run ‘:set -package types-common-journal’ to expose it.
>     (Note: this unloads all the modules in the current scope.)
>     Use -v to see a list of the files searched for.
> src/Galley/API.hs:1129: failure in expression `let run queryString = exec filterMissing (defaultRequest {queryString = queryString}) Left Right'
> expected: 
>  but got: 
>           <interactive>:23:59: error: Not in scope: ‘queryString’
> 
> Examples: 10  Tried: 1  Errors: 0  Failures: 1
> Test suite doctests failed

No idea right now.

Copy link
Contributor

@mheinzel mheinzel left a comment

Choose a reason for hiding this comment

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

Auto-detection would be really valuable to not forget adding modules, but good to get started with this.

@fisx fisx changed the title Introduce doctest. [WIP] Introduce doctest. Apr 22, 2020
@fisx fisx changed the title [WIP] Introduce doctest. Introduce doctest. Apr 23, 2020
@fisx
Copy link
Contributor Author

fisx commented Apr 23, 2020

I managed to reproduce the above error locally, by running stack test --fast from project root. So we hypothesised that it has something to do with paths, and I changed the "-isrc" in doctest.hs to an absolute path (as a way of testing the hypothesis). That appeared to fix the problem, but then when I switched back to a relative path, I couldn't reproduce the error.

I also established that stack always runs with working directory /services/galley, no matter whether I call stack test or stack test galley from repo root, or stack test . from /services/galley.

Something else is going on here. At this point I am leaning towards abandoning doctests and adding a tasty unit test suite to galley.

@mheinzel
Copy link
Contributor

Not sure what's going on either. Had a quick look at the doctest issues and there were other people having issues with different artifacts not being found, but didn't find something that would help here.
🤷

doctest has build issues and seems overall unstable and brittle.
@fisx fisx requested a review from mheinzel April 23, 2020 13:06
Copy link
Contributor

@mheinzel mheinzel left a comment

Choose a reason for hiding this comment

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

Looks good, but maybe it's a good idea to rename the PR before merging to make it clear that no doctests were added in the end.

@fisx fisx changed the title Introduce doctest. Add (very few) unit tests to galley. Apr 23, 2020
@fisx fisx merged commit 3aead6c into develop Apr 23, 2020
@fisx fisx deleted the fisx/doc-tests branch April 23, 2020 23:03
@akshaymankar akshaymankar mentioned this pull request May 7, 2020
akshaymankar added a commit that referenced this pull request May 11, 2020
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.

2 participants