Skip to content

Conversation

@hurricane642
Copy link
Contributor

This is a continuation of the work on the test library. This PR work has been done with SPARK tests. A new file base_check.py has been added, introducing the main class for the SPARK test, as well as the CSCS tests inherited from it.

@pep8speaks
Copy link

pep8speaks commented Sep 9, 2021

Hello @hurricane642, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2021-10-08 07:29:25 UTC

@jenkins-cscs
Copy link
Collaborator

Can I test this patch?

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2021

Codecov Report

Merging #2181 (21b24ec) into master (889d6e0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2181   +/-   ##
=======================================
  Coverage   85.54%   85.54%           
=======================================
  Files          55       55           
  Lines        9746     9746           
=======================================
  Hits         8337     8337           
  Misses       1409     1409           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 889d6e0...21b24ec. Read the comment docs.

@teojgo
Copy link
Contributor

teojgo commented Sep 30, 2021

Ok to test

Copy link
Contributor

@jjotero jjotero left a comment

Choose a reason for hiding this comment

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

I'm not convinced that having a name such as base_check or so is the right approach. I think the name should be more explicit of what the test does, which in this case is computing pi. For this reason, I'd suggest that we move this "base check" to hpctestlib/apps/spark/compute_pi/__init__.py change the class names as suggested below. This would allow other people to introduce other spark checks for whatever other case they want.

self.sanity_patterns = sn.assert_lt(sn.abs(pi_value - math.pi), 0.01)
self.maintainers = ['TM', 'RS']
self.tags = {'production'}
class SparkCheck(Spark_BaseCheck):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class SparkCheck(Spark_BaseCheck):
class SparkCheck(ComputePi):

@teojgo
Copy link
Contributor

teojgo commented Oct 1, 2021

@jenkins-cscs retry dom

@teojgo
Copy link
Contributor

teojgo commented Oct 8, 2021

Will continue this on #2214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants